You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@myfaces.apache.org by "Martin Koci (JIRA)" <de...@myfaces.apache.org> on 2010/02/09 21:47:28 UTC
[jira] Created: (MYFACES-2544) UIViewRoot skips uncorrectly
encodeBegin
UIViewRoot skips uncorrectly encodeBegin
----------------------------------------
Key: MYFACES-2544
URL: https://issues.apache.org/jira/browse/MYFACES-2544
Project: MyFaces Core
Issue Type: Bug
Components: JSR-314
Affects Versions: 2.0.0-beta-2
Environment: myfaces trunk
Reporter: Martin Koci
javax.faces.component.UIViewRoot.encodeBegin(FacesContext) contains:
if (!skipPhase) {
super.encodeBegin(context);
}
but skipPhase = context.getRenderResponse() || context.getResponseComplete() - it
makes sense for all phases except render response phase itself. That condition
probably should be:
if (!context.getResponseComplete()) {
super.encodeBegin(context);
}
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (MYFACES-2544) UIViewRoot skips uncorrectly
encodeBegin
Posted by "Martin Koci (JIRA)" <de...@myfaces.apache.org>.
[ https://issues.apache.org/jira/browse/MYFACES-2544?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Martin Koci updated MYFACES-2544:
---------------------------------
Status: Patch Available (was: Open)
> UIViewRoot skips uncorrectly encodeBegin
> ----------------------------------------
>
> Key: MYFACES-2544
> URL: https://issues.apache.org/jira/browse/MYFACES-2544
> Project: MyFaces Core
> Issue Type: Bug
> Components: JSR-314
> Affects Versions: 2.0.0-beta-2
> Environment: myfaces trunk
> Reporter: Martin Koci
>
> javax.faces.component.UIViewRoot.encodeBegin(FacesContext) contains:
> if (!skipPhase) {
> super.encodeBegin(context);
> }
> but skipPhase = context.getRenderResponse() || context.getResponseComplete() - it
> makes sense for all phases except render response phase itself. That condition
> probably should be:
> if (!context.getResponseComplete()) {
> super.encodeBegin(context);
> }
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Reopened: (MYFACES-2544) UIViewRoot skips uncorrectly
encodeBegin
Posted by "Leonardo Uribe (JIRA)" <de...@myfaces.apache.org>.
[ https://issues.apache.org/jira/browse/MYFACES-2544?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Leonardo Uribe reopened MYFACES-2544:
-------------------------------------
I'll revert the previous patch. The use case you have to consider is when there is an error on a beforePhase listener. If there is, context.getResponseComplete() could return true, inclusive on render response phase. Long time ago, I review MYFACES-2374 and It is known ri does not do this, and this one cause an error on a compatibility test, but in this case the previous behavior is the expected one (the javadoc says so and this one comes from jsf 1.2).
Please note maybe there is a misundersanding about what FacesContext.renderResponse() and FacesContext.getRenderResponse() do. The meaning of this constant is allow jsf to skip phases when an error occur. For example, if a validator throws a RuntimeException, ProcessUpdates and InvokeApplication phases should not be executed, so they will be skipped and RenderResponse phase should occur. Why UIViewRoot.encodeBegin skip render the view? because in jsf 1.2 this is considered an error and encodeBegin should not happen, instead, the error page is published.
Now, PreRenderViewEvent could be used to change the view to be rendered. Just take a look at org.apache.myfaces.lifecycle.RenderResponseExecutor. Its meaning and behavior are different to PreRenderComponentEvent. Maybe the real bug is UIViewRoot does not call
context.getApplication().publishEvent(context, PreRenderComponentEvent.class, UIComponent.class, this);
before call encodeBegin(), like UIComponentBase.encodeBegin() does. Maybe this one was ignored, because usually UIViewRoot does not render anything.
Note to solve this issue we have also to check if the new behavior introduced with ExceptionHandler api is compatible with the original jsf 1.2 algorithm.
Jakob, could you take a look at this one again from this point of view?
> UIViewRoot skips uncorrectly encodeBegin
> ----------------------------------------
>
> Key: MYFACES-2544
> URL: https://issues.apache.org/jira/browse/MYFACES-2544
> Project: MyFaces Core
> Issue Type: Bug
> Components: JSR-314
> Affects Versions: 2.0.0-beta-2
> Environment: myfaces trunk
> Reporter: Martin Koci
> Assignee: Jakob Korherr
> Fix For: 2.0.0-beta-2
>
> Attachments: MYFACES-2544.patch
>
>
> javax.faces.component.UIViewRoot.encodeBegin(FacesContext) contains:
> if (!skipPhase) {
> super.encodeBegin(context);
> }
> but skipPhase = context.getRenderResponse() || context.getResponseComplete() - it
> makes sense for all phases except render response phase itself. That condition
> probably should be:
> if (!context.getResponseComplete()) {
> super.encodeBegin(context);
> }
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (MYFACES-2544) UIViewRoot skips uncorrectly
encodeBegin
Posted by "Leonardo Uribe (JIRA)" <de...@myfaces.apache.org>.
[ https://issues.apache.org/jira/browse/MYFACES-2544?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12832224#action_12832224 ]
Leonardo Uribe commented on MYFACES-2544:
-----------------------------------------
The latest patch (MYFACES-2544-3.patch) is the final form of the proposed solution. If no objections I'll commit it soon
> UIViewRoot skips uncorrectly encodeBegin
> ----------------------------------------
>
> Key: MYFACES-2544
> URL: https://issues.apache.org/jira/browse/MYFACES-2544
> Project: MyFaces Core
> Issue Type: Bug
> Components: JSR-314
> Affects Versions: 2.0.0-beta-2
> Environment: myfaces trunk
> Reporter: Martin Koci
> Assignee: Jakob Korherr
> Fix For: 2.0.0-beta-2
>
> Attachments: MYFACES-2544-2.patch, MYFACES-2544-3.patch, MYFACES-2544.patch
>
>
> javax.faces.component.UIViewRoot.encodeBegin(FacesContext) contains:
> if (!skipPhase) {
> super.encodeBegin(context);
> }
> but skipPhase = context.getRenderResponse() || context.getResponseComplete() - it
> makes sense for all phases except render response phase itself. That condition
> probably should be:
> if (!context.getResponseComplete()) {
> super.encodeBegin(context);
> }
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (MYFACES-2544) UIViewRoot skips uncorrectly
encodeBegin
Posted by "Jakob Korherr (JIRA)" <de...@myfaces.apache.org>.
[ https://issues.apache.org/jira/browse/MYFACES-2544?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jakob Korherr updated MYFACES-2544:
-----------------------------------
Resolution: Fixed
Fix Version/s: 2.0.0-beta-2
Assignee: Jakob Korherr
Status: Resolved (was: Patch Available)
This is very interesting. Until now the if part in encodeBegin was never executed, because facesContext.getRenderResponse() is always true at this point. So maybe there are some unwanted side effects from the patch. Because of that I added a FIXME in the code.
Martin, maybe you could check if this is a problem with the spec. I noticed that the if path is not that different from the else path. The only difference is that a PreRenderComponentEvent is published and I don't know if this is ok. Thanks!
> UIViewRoot skips uncorrectly encodeBegin
> ----------------------------------------
>
> Key: MYFACES-2544
> URL: https://issues.apache.org/jira/browse/MYFACES-2544
> Project: MyFaces Core
> Issue Type: Bug
> Components: JSR-314
> Affects Versions: 2.0.0-beta-2
> Environment: myfaces trunk
> Reporter: Martin Koci
> Assignee: Jakob Korherr
> Fix For: 2.0.0-beta-2
>
> Attachments: MYFACES-2544.patch
>
>
> javax.faces.component.UIViewRoot.encodeBegin(FacesContext) contains:
> if (!skipPhase) {
> super.encodeBegin(context);
> }
> but skipPhase = context.getRenderResponse() || context.getResponseComplete() - it
> makes sense for all phases except render response phase itself. That condition
> probably should be:
> if (!context.getResponseComplete()) {
> super.encodeBegin(context);
> }
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (MYFACES-2544) UIViewRoot skips uncorrectly
encodeBegin
Posted by "Leonardo Uribe (JIRA)" <de...@myfaces.apache.org>.
[ https://issues.apache.org/jira/browse/MYFACES-2544?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12832226#action_12832226 ]
Leonardo Uribe commented on MYFACES-2544:
-----------------------------------------
Reading the spec javadoc, it is enforced listeners should be called from encodeBegin() and encodeEnd(). We have to use an alternate way to produce the same results.
> UIViewRoot skips uncorrectly encodeBegin
> ----------------------------------------
>
> Key: MYFACES-2544
> URL: https://issues.apache.org/jira/browse/MYFACES-2544
> Project: MyFaces Core
> Issue Type: Bug
> Components: JSR-314
> Affects Versions: 2.0.0-beta-2
> Environment: myfaces trunk
> Reporter: Martin Koci
> Assignee: Jakob Korherr
> Fix For: 2.0.0-beta-2
>
> Attachments: MYFACES-2544-2.patch, MYFACES-2544-3.patch, MYFACES-2544.patch
>
>
> javax.faces.component.UIViewRoot.encodeBegin(FacesContext) contains:
> if (!skipPhase) {
> super.encodeBegin(context);
> }
> but skipPhase = context.getRenderResponse() || context.getResponseComplete() - it
> makes sense for all phases except render response phase itself. That condition
> probably should be:
> if (!context.getResponseComplete()) {
> super.encodeBegin(context);
> }
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (MYFACES-2544) UIViewRoot skips uncorrectly
encodeBegin
Posted by "Tibor (JIRA)" <de...@myfaces.apache.org>.
[ https://issues.apache.org/jira/browse/MYFACES-2544?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12866178#action_12866178 ]
Tibor commented on MYFACES-2544:
--------------------------------
I believe there is the same issue in version 1.2 of MyFaces. Could you provide a fix also for that version? Thanks.
> UIViewRoot skips uncorrectly encodeBegin
> ----------------------------------------
>
> Key: MYFACES-2544
> URL: https://issues.apache.org/jira/browse/MYFACES-2544
> Project: MyFaces Core
> Issue Type: Bug
> Components: JSR-314
> Affects Versions: 2.0.0-beta-2
> Environment: myfaces trunk
> Reporter: Martin Koci
> Assignee: Leonardo Uribe
> Fix For: 2.0.0-beta-2
>
> Attachments: MYFACES-2544-2.patch, MYFACES-2544-3.patch, MYFACES-2544-4.patch, MYFACES-2544.patch
>
>
> javax.faces.component.UIViewRoot.encodeBegin(FacesContext) contains:
> if (!skipPhase) {
> super.encodeBegin(context);
> }
> but skipPhase = context.getRenderResponse() || context.getResponseComplete() - it
> makes sense for all phases except render response phase itself. That condition
> probably should be:
> if (!context.getResponseComplete()) {
> super.encodeBegin(context);
> }
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (MYFACES-2544) UIViewRoot skips uncorrectly
encodeBegin
Posted by "Martin Koci (JIRA)" <de...@myfaces.apache.org>.
[ https://issues.apache.org/jira/browse/MYFACES-2544?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12832185#action_12832185 ]
Martin Koci commented on MYFACES-2544:
--------------------------------------
> when there is an error on a beforePhase listener. If there is, context.getResponseComplete() could return true, inclusive on render response phase.
How is context.getResponseComplete() related to error handling? I don't get the point about error in beforePhase - if there is a error in phaseListener it is logged and/or published with exceptionHandler: "6.2.2 Backwards Compatible ExceptionHandler" and "12.3 PhaseListener"
> Please note maybe there is a misundersanding about what >FacesContext.renderResponse() and FacesContext.getRenderResponse() do. > The meaning of this constant is allow jsf to skip phases when an error > occur. For example, if a validator throws a RuntimeException,
> ProcessUpdates and InvokeApplication phases should not be executed, so they will be skipped and RenderResponse phase should occur.
Yes, but why exclude from executing exactly one method at particular class? Why UIViewRoot.encodeEnd is not exluded? UIViewRoot.encodeBegin represents the beginning of render response phase as component. It is legal for a listener (action or valueChange) to call context FacesContext.renderResponse() but it does not mean that there was a error - listener can simple be smart enough to know that no other phases except for render response are needed.
> Why UIViewRoot.encodeBegin skip render the view? because in jsf 1.2
> this is considered an error and encodeBegin should not happen,
> instead, the error page is published.
I don't think "FacesContext.getRenderResponse() true = a runtime exception" is a valid expectation - skipping to render response phase is a possible response to runtime exception, but it does not imply that render response phase is executing as response to error.
Here is example what should work:
MyUIView extends UIViewRoot {
encodeBegin() {
super.encodeBegin()
// super notifies phase listeners, puts component to EL and publish PreRenderComponentEvent
// rendering specific stuff here
}
}
This was not working with mojarra and isn't working with myfaces. But clearly it can be fixed with adding
context.getApplication().publishEvent(PreRenderComponentEvent.class) and it will work together with algorithm requested by spec.
> UIViewRoot skips uncorrectly encodeBegin
> ----------------------------------------
>
> Key: MYFACES-2544
> URL: https://issues.apache.org/jira/browse/MYFACES-2544
> Project: MyFaces Core
> Issue Type: Bug
> Components: JSR-314
> Affects Versions: 2.0.0-beta-2
> Environment: myfaces trunk
> Reporter: Martin Koci
> Assignee: Jakob Korherr
> Fix For: 2.0.0-beta-2
>
> Attachments: MYFACES-2544.patch
>
>
> javax.faces.component.UIViewRoot.encodeBegin(FacesContext) contains:
> if (!skipPhase) {
> super.encodeBegin(context);
> }
> but skipPhase = context.getRenderResponse() || context.getResponseComplete() - it
> makes sense for all phases except render response phase itself. That condition
> probably should be:
> if (!context.getResponseComplete()) {
> super.encodeBegin(context);
> }
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Resolved: (MYFACES-2544) UIViewRoot skips uncorrectly
encodeBegin
Posted by "Leonardo Uribe (JIRA)" <de...@myfaces.apache.org>.
[ https://issues.apache.org/jira/browse/MYFACES-2544?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Leonardo Uribe resolved MYFACES-2544.
-------------------------------------
Resolution: Fixed
Assignee: Leonardo Uribe (was: Jakob Korherr)
Thanks to Martin Koci for provide this patch
> UIViewRoot skips uncorrectly encodeBegin
> ----------------------------------------
>
> Key: MYFACES-2544
> URL: https://issues.apache.org/jira/browse/MYFACES-2544
> Project: MyFaces Core
> Issue Type: Bug
> Components: JSR-314
> Affects Versions: 2.0.0-beta-2
> Environment: myfaces trunk
> Reporter: Martin Koci
> Assignee: Leonardo Uribe
> Fix For: 2.0.0-beta-2
>
> Attachments: MYFACES-2544-2.patch, MYFACES-2544-3.patch, MYFACES-2544-4.patch, MYFACES-2544.patch
>
>
> javax.faces.component.UIViewRoot.encodeBegin(FacesContext) contains:
> if (!skipPhase) {
> super.encodeBegin(context);
> }
> but skipPhase = context.getRenderResponse() || context.getResponseComplete() - it
> makes sense for all phases except render response phase itself. That condition
> probably should be:
> if (!context.getResponseComplete()) {
> super.encodeBegin(context);
> }
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (MYFACES-2544) UIViewRoot skips uncorrectly
encodeBegin
Posted by "Leonardo Uribe (JIRA)" <de...@myfaces.apache.org>.
[ https://issues.apache.org/jira/browse/MYFACES-2544?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12832211#action_12832211 ]
Leonardo Uribe commented on MYFACES-2544:
-----------------------------------------
>How is context.getResponseComplete() related to error handling?
Look org.apache.myfaces.renderkit.ErrorPageWriter. This one is called after log a page.
>Yes, but why exclude from executing exactly one method at particular class? Why UIViewRoot.encodeEnd is not exluded?
This should not happen and it is a bug. This means the code that call the listener inside encodeBegin() should be called overriding UIViewRoot.encodeAll(). Note this is not notice because the typical use case fall into previous phases.
>I don't think "FacesContext.getRenderResponse() true = a runtime exception" is a valid expectation
True. My intention here is analize a possible use case that makes fail the previous patch, not say this is the rule.
I committed an incomplete alternate patch. It is better to check:
context.getResponseComplete() || (context.getRenderResponse() && !PhaseId.RENDER_RESPONSE.equals(phaseId))
and move the algorithm that check for the listener to encodeAll. This fix should be done in jsf 1.2 too. I'll commit the full solution soon.
> UIViewRoot skips uncorrectly encodeBegin
> ----------------------------------------
>
> Key: MYFACES-2544
> URL: https://issues.apache.org/jira/browse/MYFACES-2544
> Project: MyFaces Core
> Issue Type: Bug
> Components: JSR-314
> Affects Versions: 2.0.0-beta-2
> Environment: myfaces trunk
> Reporter: Martin Koci
> Assignee: Jakob Korherr
> Fix For: 2.0.0-beta-2
>
> Attachments: MYFACES-2544-2.patch, MYFACES-2544.patch
>
>
> javax.faces.component.UIViewRoot.encodeBegin(FacesContext) contains:
> if (!skipPhase) {
> super.encodeBegin(context);
> }
> but skipPhase = context.getRenderResponse() || context.getResponseComplete() - it
> makes sense for all phases except render response phase itself. That condition
> probably should be:
> if (!context.getResponseComplete()) {
> super.encodeBegin(context);
> }
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (MYFACES-2544) UIViewRoot skips uncorrectly
encodeBegin
Posted by "Martin Koci (JIRA)" <de...@myfaces.apache.org>.
[ https://issues.apache.org/jira/browse/MYFACES-2544?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12831966#action_12831966 ]
Martin Koci commented on MYFACES-2544:
--------------------------------------
This is probably bug in spec. https://javaserverfaces.dev.java.net/nonav/docs/2.0/javadocs/javax/faces/component/UIViewRoot.html says: "Upon return from the listener, call FacesContext.getResponseComplete() and FacesContext.getRenderResponse(). If either return true set the internal state flag to true. ... Execute any processing for this phase if the internal state flag was not set." But this nonsence for render response phase, because FacesContext.renderResponse() = "Signal the JavaServer faces implementation that, as soon as the current phase of the request processing lifecycle has been completed, control should be passed to the <em>Render Response</em> phase, bypassing any phases that have not been executed yet."
Delivering PreRenderComponentEvent is ok, because spec says:
"encodeBegin() must publish a PreRenderComponentEvent" or "PreRenderComponentEvent indicates that the source component is about to be rendered". UIViewRoot is UIComponent too and there is no reason to act differently (although PreRenderViewEvent and PreRenderComponentEvent attached to UIViewRoot have same meaning).
> UIViewRoot skips uncorrectly encodeBegin
> ----------------------------------------
>
> Key: MYFACES-2544
> URL: https://issues.apache.org/jira/browse/MYFACES-2544
> Project: MyFaces Core
> Issue Type: Bug
> Components: JSR-314
> Affects Versions: 2.0.0-beta-2
> Environment: myfaces trunk
> Reporter: Martin Koci
> Assignee: Jakob Korherr
> Fix For: 2.0.0-beta-2
>
> Attachments: MYFACES-2544.patch
>
>
> javax.faces.component.UIViewRoot.encodeBegin(FacesContext) contains:
> if (!skipPhase) {
> super.encodeBegin(context);
> }
> but skipPhase = context.getRenderResponse() || context.getResponseComplete() - it
> makes sense for all phases except render response phase itself. That condition
> probably should be:
> if (!context.getResponseComplete()) {
> super.encodeBegin(context);
> }
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (MYFACES-2544) UIViewRoot skips uncorrectly
encodeBegin
Posted by "Leonardo Uribe (JIRA)" <de...@myfaces.apache.org>.
[ https://issues.apache.org/jira/browse/MYFACES-2544?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12832238#action_12832238 ]
Leonardo Uribe commented on MYFACES-2544:
-----------------------------------------
The latest patch (MYFACES-2544-4.patch) uses encodeBegin() and encodeEnd(), but before execute encodeChildren() or encodeEnd() it check for getResponseComplete(). If no objections, I'll commit this code soon.
> UIViewRoot skips uncorrectly encodeBegin
> ----------------------------------------
>
> Key: MYFACES-2544
> URL: https://issues.apache.org/jira/browse/MYFACES-2544
> Project: MyFaces Core
> Issue Type: Bug
> Components: JSR-314
> Affects Versions: 2.0.0-beta-2
> Environment: myfaces trunk
> Reporter: Martin Koci
> Assignee: Jakob Korherr
> Fix For: 2.0.0-beta-2
>
> Attachments: MYFACES-2544-2.patch, MYFACES-2544-3.patch, MYFACES-2544-4.patch, MYFACES-2544.patch
>
>
> javax.faces.component.UIViewRoot.encodeBegin(FacesContext) contains:
> if (!skipPhase) {
> super.encodeBegin(context);
> }
> but skipPhase = context.getRenderResponse() || context.getResponseComplete() - it
> makes sense for all phases except render response phase itself. That condition
> probably should be:
> if (!context.getResponseComplete()) {
> super.encodeBegin(context);
> }
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.