tomahawk's panelNavigation2 doesn't keep state when navigating between views

• Key: SHALE-316
• Type: Bug
• Status: Resolved
• Resolution: Fixed
• Priority: Major
• Assignee: Gary VanMatre
• Reporter: Tom Pasierb
• Component/s: Clay
• Affects Version/s: 1.0.0, 1.0.1, 1.0.2, 1.0.3, 1.0.4-SNAPSHOT
• Fix Version/s: 1.0.4

Description

I prepared an example which is attached to the issue entry. Use maven to build the war or exploded app directory and run it. It should start fine. I described briefly what goes wrong and what causes it in my opinion.

Basically clay behaves differently if the order in which the panelNavigation2 and symbols are placed in the template file changes. This probably doesn't explain much - just look at the example.

I hope that the example I provided will be enough to correct the problem ;-)

Regards,

Comments

[Gary VanMatre]

Tom, I think that this issues was resolved with some changes I made yesterday. Both layouts rendered as expected - unless I'm missing something.

GOOD LAYOUT:
content
      header
      main
          navigation
          content
      footer

BAD LAYOUT:
content
     header
     main
         content
         navigation
     footer

   
The main content and navigation markup seem to be rendered in the correct div tags.
Please try pulling down the latest clay source. You will need to make a couple changes due to the recent reorg.

web.xml

<filter>
  <filter-name>shale</filter-name>
  <filter-class>org.apache.shale.application.faces.ShaleApplicationFilter</filter-class>
</filter>


pom.xml

<dependency>
   <groupId>org.apache.shale</groupId>
   <artifactId>shale-application</artifactId>
   <version>1.0.4-SNAPSHOT</version>
</dependency>

BTW, this is a very cool example.

[Gary VanMatre]

I noticed that the JSP example has a different layout (bdefault.faces)

content
       header
       main
            content
       navigation
       footer

There is a problem with the header.html template. The problem has to do with the support for the "f:verbatim" within clay templates. When documents that have namespaces are used to build the component tree, any text that is under (child of) a namespace node that is not well-formed, is removed. The markup parser treats newlines and white space as not well-formed nodes so that's why they are removed.

I'll try to find a fix for this problem but in the mean time, use a output text.
<span jsfid="void"
      xmlns="http://www.w3.org/1999/xhtml"
xmlns:h="http://java.sun.com/jsf/html"
xmlns:f="http://java.sun.com/jsf/core"
xmlns:t="http://myfaces.apache.org/tomahawk"
xmlns:clay="http://shale.apache.org/clay">

     HEADER - here goes a link to "home page" but nothing gets rendered as the body of the a tag
        <h:outputLink value="#{facesContext.externalContext.requestContextPath}/index.jsp"><h:outputText value="home"/></h:outputLink>

</span>

*** OR ***

Just put the text in a well-formed node like a <span/>

<span jsfid="void"
      xmlns="http://www.w3.org/1999/xhtml"
xmlns:h="http://java.sun.com/jsf/html"
xmlns:f="http://java.sun.com/jsf/core"
xmlns:t="http://myfaces.apache.org/tomahawk"
xmlns:clay="http://shale.apache.org/clay">

     HEADER - here goes a link to "home page" but nothing gets rendered as the body of the a tag
        <h:outputLink value="#{facesContext.externalContext.requestContextPath}/index.jsp><span>home<span/></h:outputLink>

</span>

[Gary VanMatre]

The verbatim body problem discussed previously is fixed in the 20061021 nightly build.

The following will now work.

<h:outputLink value="#{facesContext.externalContext.requestContextPath}/index.jsp><f:verbatim>home</f:verbatim></h:outputLink>

[Tom Pasierb]

an updated version of the example app

[Gary VanMatre]

Tom, I was able to recreate the problem. The reason why one layout works over the other has to do with where the dynamic part of the tree is located. The layout that works places the navigation panel before the variable body. The layout that doesn't work places the variable body content before the navigation.

One works over the other because of how the HtmlPanelNavigationMenu works. On the decode of this component, it saves the current view root in the request. The next view is created or restored and the rendering invoked on the HtmlPanelNavigationMenu. The previous view root is pulled from request scope. The logic looks to see if it can find the current component by client id in the previous view root. This fails because the dynamic content is before the navigation.

The Clay component is a NamingContainer. The layers of template inclusion are handled in a Clay full view by nesting the Clay component as a child of itself. This is why the client id's are so deep when using the full XML or HTML views.

This is not really a Clay issue but more of an issue of the strategy that the HtmlPanelNavigationMenu component uses to determine the previous state.

org.apache.myfaces.custom.navmenu.htmlnavmenu.HtmlPanelNavigationMenu:

public void encodeBegin(FacesContext context) throws IOException {
...
...
...
UIViewRoot previousRoot = (UIViewRoot)context.getExternalContext().getRequestMap().get(PREVIOUS_VIEW_ROOT);
if (previousRoot != null) {
    if (previousRoot.findComponent(getClientId(context)) instanceof HtmlPanelNavigationMenu) {
                    restoreOpenActiveStates(context, previousRoot, getChildren());
     }
}


When used in JSP, the naming containers are more controlled but I think we could break this component in JSP by adding a subview to one of the templates.

<f:subview id="bpage2">
   <jsp:include page="/no_clay/bmenu.jsp" />
</f:subview>


I think this component would be better off it required id's for the commandNavigation2 components. It could the recursively search the previous tree for a component with a matching id (not clientId).

If you are ok with this response, please change the status to resolved. I did fix the other issue with the header template. The second fix that really cleaned up that namespace verbatim was on 10/21.

This is a great example. Would you consider cleaning it up and submitting it as an addition to the shale-apps?

[Tom Pasierb]

OK. So you're saying that it is actually a matter of how paneNavigation2 saves and restores its state, right? And there is nothing clay can do solve this.

1. do you think an issue should be created for this on the myfaces/tomahawk jira?

2. What would you suggest for a navigation component, something similar to panelNavigation2 that would work with the "layout_bad.html" layout? I've got this html template I want to use where the navigation div is placed almost at the end of the file, which causes the problem with panelNavigation2 and clay :-(

[Tom Pasierb]

one more thing:
3. even if I swapped the divs so that navigation div was before the content div but there was/were other divs above it (i.e. header) with variable content this would still break the state saving/restoring functionality of panelNavigation2, right?

[Gary VanMatre]

>1. do you think an issue should be created for this on the myfaces/tomahawk jira?

Yes, I agree. We should take it up on tomahawk's jira.

>2. What would you suggest for a navigation component, something similar to panelNavigation2 that would work with the "layout_bad.html" layout? I've got this html template I want to use where the navigation div is placed almost at the end of the file, which causes the problem with panelNavigation2 and clay :-(

It's really has a greater scope that just Clay. It's pretty easy to recreate in JSP. But, I would suggest that the component try harder to find itself in the previous view. Maybe if the first call fails using clientId's, try using the component id by scanning the tree.

    private UIComponent findComponent(UIComponent parent, String id) {
        if (parent == null) {
            return null;
        }

        if (parent.getId() != null && parent.getId().equals(id)) {
            return parent;
        } else {
            Iterator ci = parent.getChildren().iterator();
            while (ci.hasNext()) {
                UIComponent child = (UIComponent) ci.next();
                UIComponent target = findComponent(child, id);
                if (target != null) {
                    return target;
                }
            }
        }

        return null;
    }



>3. even if I swapped the divs so that navigation div was before the content div but there was/were other divs above it (i.e. header) with variable content this would still break the state saving/restoring functionality of panelNavigation2, right?

Yeah, I think you are right. Adding content would through off the generated id's.

[Gary VanMatre]

The more I think about this, I wonder if it is wrong to force naming containers. Maybe we should create a little Clay that is not a naming container. Make it the root of the HTML templates. You could then choose if you wanted little clay or big clay.

Not sure what we would call it (mud, silt :-)?

[Gary VanMatre]

Tom, I removed the requirement that Clay is a naming container. This will should bring the Clay constructed html views more inline with JSP created views and solves this issue.

I will try it today, thanks.

However it would probably still be desirable to make it an issue of Tomahawk's panelNavigation2 - do you think I could just use a part of one of your previous posts as an explanation while creating an issue for this in tomahawk JIRA? Or maybe you could create an issue - you definietly know what you're talking about :-) and I'm just a beginner and don't know all those inner workings of jsf ;-)

[Gary VanMatre]

Ahh, you are doing great. I'll submit a patch in the next couple of days for the component. Thanks for your help. I think the Clay changes has us pointed down the right path too.

[Gary VanMatre]

Opened a JIRA issue with myfaces tomahawk https://issues.apache.org/jira/browse/TOMAHAWK-755

[Tom Pasierb]

   [[ Old comment, sent from unregistered email on Sat, 21 Oct 2006 19:37:02 +0200 ]]



ok, I missed that. Apparently div all ids should be unique. That also
aplies to the pure jsp example. And in all cases I wanted to put
"content" and "navigation" divs in main div.
i'll give it a try.

If it still doen't work I'll upload an updated example.

----------------------------------------------------------------------
PS. >>> http://link.interia.pl/f19a6

[Tom Pasierb]

   [[ Old comment, sent from unregistered email on Sat, 21 Oct 2006 21:08:02 +0200 ]]

i'll try to add my comment tomorrow as JIRA seems to be down right now.

Regards


----------------------------------------------------------------------
PS. >>> http://link.interia.pl/f19a6

[Gary VanMatre]

Well, you are doing great. I'll post a patch the next couple of days and close out this one.
Thanks for the help. I think the Clay changes has us pointed in the right direction.

Gary