dev@javaserverfaces.java.net

Re: REVIEW REQUEST: Issue 723

From: Ryan Lubke <Ryan.Lubke_at_Sun.COM>
Date: Thu, 10 Apr 2008 09:20:26 -0700

Jason Lee wrote:
> I have attached a proposed fix for issue 723. Please review at your
> convenience. It's an easy one. :)
>
> https://javaserverfaces.dev.java.net/issues/show_bug.cgi?id=723
>
> --
> Jason Lee, SCJP
> Software Architect -- Objectstream, Inc.
> Mojarra and Mojarra Scales Dev Team
> https://mojarra.dev.java.net
> https://scales.dev.java.net
> http://blogs.steeplesoft.com
I *think* there is a bug in here that can be fixed while you've got the
source modified.

I've applied your patch and looking at, say, UIViewRoot.processDecodes()
we see:

1 skipPhase = false;
2 // avoid creating the PhaseEvent if possible by doing redundant
3 // null checks.
4 if (null != beforePhase || null != phaseListeners) {
5 notifyPhaseListeners(context,
PhaseId.APPLY_REQUEST_VALUES, true);
6 }
7 if (!skipPhase) {
8 super.processDecodes(context);
9 broadcastEvents(context, PhaseId.APPLY_REQUEST_VALUES);
10 }
11 clearFacesEvents(context);
12
13 // avoid creating the PhaseEvent if possible by doing redundant
14 // null checks.
15 if (null != beforePhase || null != phaseListeners) {
16 notifyPhaseListeners(context, PhaseId.APPLY_REQUEST_VALUES,
false);
17 }


Look at line 15 looks a bit off. I'm pretty sure that null check was
meant to be null != afterPhase.
That cut and paste error in the other related lifecycle methods as well.

Since we're eliminating duplicate code, lines 2-6 could be refactored into
it's own method, notifyBefore(), and lines 13-17 could be moved into
notifyAfter() (or something similar), but will leave that up to you.