Details
-
Type: Improvement
-
Status: Closed
-
Priority: Major
-
Resolution: Fixed
-
Affects Version/s: 1.8.1
-
Component/s: ICE-Components
-
Labels:None
-
Environment:ICEfaces
Description
The recommended fix is to remove the attach() functionality and update the CommandLinkRenderer to a pure ResponseWriter implementation.
Activity
- All
- Comments
- History
- Activity
- Remote Attachments
- Subversion
Please survey all of our standard-extended components to see if they use attach also.
The real call is DOMContext.attachDOMContext() and it came across that almost all components are using it.
This is true, however the actual problem is the state that the DOM is in at the moment of the attachDOMContext() call. See the comment here in the implementation of the attachDOMContext() method:
//context may have been severed from the tree at some point
if (context.isInitialized()) {
if (!(cursorParent instanceof Element))
//TODO: Remove as per ICE-4088
//both MenuBarRenderer.encodeEnd and
//CommandLinkRenderer.encodeBegin make use of the
//attach method and should be fixed
context.attach((Element) cursorParent);
}
One procedure for fixing this bug would be to throw an exception from the attach() method so that the components could be verified as fixed, then remove the code from DOMContext.
I dump the stack trace from the context.attach() and found that the following components hitting the context.attach() method.
inputSecret
selectOneMenu
selectManyMenu
selectOneListbox
selectManyListbox
MenuBar
outputLabel
outputChart
panelCollapsible
panelDivider
panelSeries
PanelStack
Rest of the components do not hit context.attach() .
The context.attach() being conditionally called in the DOMContext.attachDOMContext(). Here is the code snippet:
public static DOMContext attachDOMContext(..) {
......
if (context.isInitialized())
}
We can see the method gets invoked when context is initialized.
- What sets initialize to "true"?
The initialize property being set to true, when renderer calls DOMContext.setRootNode();
- How many times renderer invokes the DOMContext.attachDOMContext()?
Only once.
- In what sequence renderer calls these methods?
DOMContext.attachDOMContext() //initialize is false
DOMContext.setRootNode() //set initialize to true
By looking at above call sequence it seems like the context.attach( ) should never be called.
- Why context.attach() is being called then?
All of our components extends DOMBasicRenderer, which defines the encodeEnd method please see the snippet below.
public void encodeEnd(...) { renderEnd(..); //renderer overrides it. DOMContext domContext = DOMContext.attachDOMContext(facesContext, uiComponent); domContext.stepOver(); }
AS you can see it makes a call of attachDOMContext. So any of the component which doesn't override its encodeEnd ends up calling attachDOMContext twice, which will cause context.attach( ) to be called.
That's interesting; is it just a matter of implementing encodeEnd() then?
Yes, implementing encodeEnd() would prevent invoking attachDOMContext twice but the encodeEnd has some effect and message related code applies to all component.
I think we should remove the following two lines from the DOMBasicRenderer.encodeEnd
DOMContext domContext = DOMContext.attachDOMContext(facesContext, uiComponent);
domContext.stepOver();
and if stepOver() is required we should use getDOMContext instead:
DOMContext domContext = DOMContext.getDOMContext(facesContext, uiComponent);
domContext.stepOver();
That looks good.
The following complete list of components is generated using the component-showcase, all of them reach context.attach():
class --->>>> class com.icesoft.faces.component.outputconnectionstatus.OutputConnectionStatus
class --->>>> class com.icesoft.faces.component.ext.HtmlSelectOneMenu
class --->>>> class com.icesoft.faces.component.panelcollapsible.PanelCollapsible
class --->>>> class com.icesoft.faces.component.panelstack.PanelStack
class --->>>> class com.icesoft.faces.component.ext.HtmlOutputLabel
class --->>>> class com.icesoft.faces.component.ext.HtmlSelectManyMenu
class --->>>> class com.icesoft.faces.component.ext.HtmlSelectOneListbox
class --->>>> class com.icesoft.faces.component.ext.HtmlSelectManyListbox
class --->>>> class com.icesoft.faces.component.ext.HtmlMessage
class --->>>> class com.icesoft.faces.component.ext.HtmlInputSecret
class --->>>> class com.icesoft.faces.component.selectinputtext.SelectInputText
class --->>>> class com.icesoft.faces.component.ext.HtmlOutputLabel
class --->>>> class com.icesoft.faces.component.outputchart.OutputChart
class --->>>> class com.icesoft.faces.component.outputresource.OutputResource
class --->>>> class com.icesoft.faces.component.panelseries.PanelSeries
class --->>>> class com.icesoft.faces.component.gmap.GMap
class --->>>> class com.icesoft.faces.component.menubar.MenuItem
class --->>>> class com.icesoft.faces.component.menubar.MenuBar
class --->>>> class com.icesoft.faces.component.menupopup.MenuPopup
class --->>>> class com.icesoft.faces.component.menubar.MenuItem
class --->>>> class com.icesoft.faces.component.menupopup.MenuPopup
class --->>>> class com.icesoft.faces.component.inputrichtext.InputRichText
class --->>>> class com.icesoft.faces.component.paneldivider.PanelDivider
class --->>>> class com.icesoft.faces.component.panelpositioned.PanelPositioned
Changing "attachDOMContext" to "getDOMContext" trims down the list as follows:
class --->>>> class com.icesoft.faces.component.ext.HtmlSelectOneMenu
class --->>>> class com.icesoft.faces.component.ext.HtmlOutputLabel
class --->>>> class com.icesoft.faces.component.ext.HtmlSelectManyMenu
class --->>>> class com.icesoft.faces.component.ext.HtmlSelectOneListbox
class --->>>> class com.icesoft.faces.component.menupopup.MenuPopup
class --->>>> class com.icesoft.faces.component.ext.HtmlSelectManyListbox
class --->>>> class com.icesoft.faces.component.panelseries.PanelSeries
class --->>>> class com.icesoft.faces.component.panelpositioned.PanelPositioned
The problem with attach() is actually the following code, so if components do not reach this, the method can be removed:
if (rootNode.getParentNode() != cursorParent) {
try
catch (DOMException e)
{ //this happens in strea-write mode only. }}
The real problem is the DOMException that is ignored, so for work on this bug, the exception should be printed.
There isn't any logger defined for this class, so I just Sysout the exception and removed all my changes. When I tested the component-showcase only menuPopup is throwing this.
class --->>>> class com.icesoft.faces.component.menupopup.MenuPopup
org.w3c.dom.DOMException: HIERARCHY_REQUEST_ERR: An attempt was made to insert a node where it is no
t permitted.
Modified: D:\work\development\head\svn\ossrepo\icefaces\trunk\icefaces\component\src\com\icesoft\faces\component\menupopup\MenuPopupRenderer.java
Modified: D:\work\development\head\svn\ossrepo\icefaces\trunk\icefaces\component\src\com\icesoft\faces\component\panelpositioned\PanelPositionedRenderer.java
Modified: D:\work\development\head\svn\ossrepo\icefaces\trunk\icefaces\component\src\com\icesoft\faces\component\panelseries\PanelSeriesRenderer.java
Modified: D:\work\development\head\svn\ossrepo\icefaces\trunk\icefaces\core\src\com\icesoft\faces\renderkit\dom_html_basic\DomBasicRenderer.java
Modified: D:\work\development\head\svn\ossrepo\icefaces\trunk\icefaces\core\src\com\icesoft\faces\renderkit\dom_html_basic\LabelRenderer.java
Modified: D:\work\development\head\svn\ossrepo\icefaces\trunk\icefaces\core\src\com\icesoft\faces\renderkit\dom_html_basic\MenuRenderer.java
Sending content: D:\work\development\head\svn\ossrepo\icefaces\trunk\icefaces\core\src\com\icesoft\faces\renderkit\dom_html_basic\MenuRenderer.java
Sending content: D:\work\development\head\svn\ossrepo\icefaces\trunk\icefaces\component\src\com\icesoft\faces\component\panelseries\PanelSeriesRenderer.java
Sending content: D:\work\development\head\svn\ossrepo\icefaces\trunk\icefaces\component\src\com\icesoft\faces\component\panelpositioned\PanelPositionedRenderer.java
Sending content: D:\work\development\head\svn\ossrepo\icefaces\trunk\icefaces\core\src\com\icesoft\faces\renderkit\dom_html_basic\LabelRenderer.java
Sending content: D:\work\development\head\svn\ossrepo\icefaces\trunk\icefaces\component\src\com\icesoft\faces\component\menupopup\MenuPopupRenderer.java
Sending content: D:\work\development\head\svn\ossrepo\icefaces\trunk\icefaces\core\src\com\icesoft\faces\renderkit\dom_html_basic\DomBasicRenderer.java
Completed: At revision: 19064
Changes made to the renderers to make sure components are calling getDOMContext to get the DOMContext after initialization and the DOMException being printed.
I confirmed that the context.attach() never gets hit by any component.
Please implement CommandLinkRenderer without DOMContext .