Incompatibility between Primefaces Dialog Framework and Omnifaces extension-less URLs

[GoogleCodeExporter]

I discovered an incompatibility of the PrimeFaces 4.0 dialog framework and 
extension-less URLs by Omnifaces 1.4.1. Since I am not sure on whose side is 
the defect, I am reporting it on both sides in hopes to keep the features of 
these two great projects compatible.

Issue in Primefaces: https://code.google.com/p/primefaces/issues/detail?id=5646
Please see attached stack-trace, screenshot, and test maven project to 
reproduce. Alternatively please see this SO question for detailed code: 
http://stackoverflow.com/questions/16672415/omnifaces-extensionless-urls-interfe
re-with-primefaces-dialog-framework

The issue is that RequestContext.getCurrentInstance().returnFromDialog() throws 
a NPE when Omnifaces extension-less URLs are enabled. In the screenshot the 
error occurs after hitting the return value button. The expected output is that 
the dialog disappears and the value is returned to the backing bean.

Run on GlassFish Server Open Source Edition 3.1.2.2 (build 5), Mojarra 2.1.6 
(SNAPSHOT 20111206).

I narrowed down the issue to the fact that because 
`org.omnifaces.facesviews.FacesViewsForwardingFilter` calls 
`application.setViewHandler(new 
FacesViewsViewHandler(application.getViewHandler()));` Then the 
`org.omnifaces.facesviews.FacesViewsViewHandler` never calls through to the 
wrapped `org.primefaces.application.DialogViewHandler`. Therefore the `dcid` 
parameter is never added to the request parameter map.

Regards,
Radu

Original issue reported on code.google.com by rdcrng on 21 May 2013 at 4:04

[GoogleCodeExporter]

Thank you for reporting.

I believe the FacesViewsViewHandler#getActionURL() needs to be altered to 
always call super.getActionURL(context, viewId) and then make manipulations on 
the return value if necessary.

Arjan, can you take a look at this?

Original comment by balusc on 21 May 2013 at 4:09

[GoogleCodeExporter]

Here's a diff of what I implemented for myself for now against the latest 
version of Omnifaces:

# FacesViewsViewHandler.java
--- Base (BASE)
+++ Locally Modified (Based On LOCAL)
@@ -19,6 +19,7 @@
 import static org.omnifaces.util.Faces.getRequestAttribute;
 import static org.omnifaces.util.ResourcePaths.isExtensionless;
 import static org.omnifaces.util.ResourcePaths.stripExtension;
+import static org.omnifaces.util.ResourcePaths.getQueryParameters;

 import java.util.Map;

@@ -45,19 +46,20 @@

    @Override
    public String getActionURL(FacesContext context, String viewId) {
-
+            String actionURL = super.getActionURL(context, viewId);
+            String queryParameters = getQueryParameters(actionURL);
        Map<String, String> mappedResources = getApplicationAttribute(context, FACES_VIEWS_RESOURCES);
        if (mappedResources.containsKey(viewId)) {

            if (isScannedViewsAlwaysExtensionless(context) || isOriginalViewExtensionless(context)) {
                // User has requested to always render extensionless, or the requested viewId was mapped and the current
                // request is extensionless, render the action URL extensionless as well.
-               return context.getExternalContext().getRequestContextPath() + 
stripExtension(viewId);
+               return context.getExternalContext().getRequestContextPath() + 
stripExtension(viewId) + queryParameters;
            }
        }

        // Not a resource we mapped or not a forwarded one, let the original view handler take care of it.
-       return super.getActionURL(context, viewId);
\ No newline at end of file
+       return actionURL;
\ No newline at end of file
    }

    private boolean isOriginalViewExtensionless(FacesContext context) {

# ResourcePaths.java
--- Base (BASE)
+++ Locally Modified (Based On LOCAL)
@@ -101,4 +101,19 @@
        return filteredResources;
    }

+        /**
+    * Extracts the query string from a resource.
+    *
+    * @param resource A URL string
+    * @return the query string part of the URL
+    */
+        public static String getQueryParameters(final String resource) {
+            String queryParameters = "";
+            int questionMark = resource.indexOf('?');
+            if (questionMark != -1) {
+                queryParameters = resource.substring(questionMark);
 }
+            return queryParameters;
+        }
+
+}

Original comment by rdcrng on 21 May 2013 at 4:32

[GoogleCodeExporter]

Guys, sorry for taking a bit longer to get back to you.

Indeed, the view handler should delegate to its parent handler and work with 
the result. In general the parent view handler can do whatever it wants with 
the view id, maybe exchanging it for another view and path altogether. I wasn't 
quite sure how to handle this before so I didn't handle it at all for the 
moment.

What I did now is checking the returned URL against the known FacesServlet 
extension mappings + the extension of the given view id. If prefix mapping is 
used, the action URL will most likely contain the view id extension and if 
suffix mapping is used it should contain one of the known FacesServlet mappings.

Unfortunately this doesn't work for Servlet 2.5 users now, so I'll have to 
think of something for that. Maybe the diff as given by rdcrng is a good 
alternative for that environment.

See 
https://code.google.com/p/omnifaces/source/detail?r=f3c434e1fb423e21c136f93e9099
55ef8af1b8d2 for the changes thus far.

Original comment by arjan.tijms on 27 May 2013 at 9:54

[GoogleCodeExporter]

Added rdcrng's idea as an alternative mode and made that the default for 
Servlet 2.5 in 
https://code.google.com/p/omnifaces/source/detail?r=ebb5ccb30ade7eb23cf0b1d1c1a7
fd904187a5cd

Original comment by arjan.tijms on 27 May 2013 at 10:45

[GoogleCodeExporter]

Arjan, I tested your changes against my project and it works as expected - 
thanks.

Original comment by rdcrng on 28 May 2013 at 2:47

[GoogleCodeExporter]

rdcrng, great it works :) I'll close the issue then. Thanks again for reporting 
this!

Original comment by arjan.tijms on 28 May 2013 at 9:28

• Changed state: Fixed

[GoogleCodeExporter]

Glad I could be of help and thanks for the credit :)

Original comment by rdcrng on 29 May 2013 at 11:38