javaee / mojarra

PLEASE NOTE: This project has moved to Eclipse Foundation and will be archived under the JavaEE GitHub Organization. After Feb. 1, 2021, the new location will be github.com/javaee/mojarra. Mojarra - Oracle's implementation of the JavaServer Faces specification
https://github.com/eclipse-ee4j/mojarra
Other
164 stars 58 forks source link

Composite component of contract with localePrefix fails to load #3432

Closed javaserverfaces closed 9 years ago

javaserverfaces commented 10 years ago

We have a composite component inside a contract. The composite component has no default (Locale?). When there is a localePrefix, the composite component is not found. For example: A contract named 'mobile' contains a composite component which does not exist in the default contract. The localePrefix is set to some value like 'lp'. The composite component is referenced from a facelet in the 'mobile' contract. JSF produces the following error: /contracts/mobile/index.xhtml @19,100 Tag Library supports namespace: http://java.sun.com/jsf/composite/extra, but no tag was defined for name: mobileOnly

Environment

glassfish 3.1.2.2 mojarra 2.2.8

Affected Versions

[2.2.8]

javaserverfaces commented 10 years ago

Reported by llorenzen

javaserverfaces commented 10 years ago

llorenzen said: Example project (i am not allowed to attach things): https://github.com/larslorenzen/JAVASERVERFACES-3428

javaserverfaces commented 10 years ago

@manfredriem said: Please send the reproducer to issues@javaserverfaces.java.net. Thanks!

javaserverfaces commented 10 years ago

@manfredriem said: Can you please try to reproduce it on GF 4.1?

javaserverfaces commented 10 years ago

llorenzen said: This bug is reproducible in glassfish 4.1 (mojarra 2.2.7)

javaserverfaces commented 10 years ago

@manfredriem said: With JSF 2.2 if you have an active localePrefix the composite component path needs to include the localePrefix, so in your example it should be lp/extra/mobileOnly.xhtml instead of just extra/mobileOnly.xhtml.

Once you change your example to reflect this you will see it works. So closing as "Works as Designed".

javaserverfaces commented 10 years ago

llorenzen said: Composite components are hardly usable with contracts if i have to duplicate my components for every locale prefix (we have 12 in our current application). Shouldn't there be at least some kind of fallback to the composite component resource without the locale prefix (the same like for 'normal' css, js, etc. resources)?

javaserverfaces commented 10 years ago

@manfredriem said: I understand, but as this could potentially be a major performance hit this was tabled for JSF 2.3. Feel free to file a spec issue if you feel it should be doing something similar as ResourceBundle does.

javaserverfaces commented 10 years ago

llorenzen said: The FaceletWebappResourceHelper already gets called twice: first with localePrefix=something second with localePrefix=null if you change

path = library.getPath() + "/" + resourceName;

to

path = library.getPath(localePrefix) + "/" + resourceName;

like it is done in WebappResourceHelper and ClassPathResourceHelper, i think it would do the fallback.

Also in findLibrary(): include the localePrefix in the calculated path.

javaserverfaces commented 10 years ago

cappi said: The spec has a typo here, but there is a comment in the pseudocode, which clearly states that all resources should work with locale prefixes and contracts:

function deriveResourceIdConsideringLocalePrefixAndContracts(prefix, resourceName, libraryName) {
  var localePrefix = getLocalePrefix();
  for (var contract : getLibraryContracts()) {
    var result = deriveResourceId(contract, prefix, resourceName, libraryName,
localePrefix);
    // If the application has been configured to have a localePrefix, and the resource
    // is not found, try to find it again, without the localePrefix.
    if (null == result && null != localePrefix) {
      result = deriveResourceId(prefix, resourceName, libraryName, null);
    }
    if (null != result) {
      return result;
    }
  }
  // try without a contract
  var result = deriveResourceId(null, prefix, resourceName, libraryName, localePrefix);
  if (null == result && null != localePrefix) {
    result = deriveResourceId(null, prefix, resourceName, libraryName, null);
  }
}

The line

result = deriveResourceId(prefix, resourceName, libraryName, null);

is obviously wrong. It must be

result = deriveResourceId(contract, prefix, resourceName, libraryName, null)

So we have an implementation and a spec issue.

javaserverfaces commented 10 years ago

@manfredriem said: Frank, I have assigned it to you. Can you have a look at this one? Thanks!

javaserverfaces commented 10 years ago

cappi said: I'll take a look at it after my holidays. Seems a bigger issue.

javaserverfaces commented 10 years ago

cappi said: The problem was the faceletResourceHelper, which always responded, even if no view resource was requested. I changed it to only handle view resources.

Manfred, can you please take a look at the patch (for latest MOJARRA_2_2X_ROLLING).

If it is OK, can you apply it and close the issue?

javaserverfaces commented 10 years ago

@manfredriem said: r=mriem, but can you apply it to 2.3 trunk. Once that completes a back port issue should be created. Thanks!

javaserverfaces commented 10 years ago

cappi said: Committed to the trunk (revision 13954).

javaserverfaces commented 10 years ago

@manfredriem said: Hi Frank,

Unfortunately I will have to revert it from the trunk, the following test is failing com.sun.faces.application.resource.TestResourceImpl.testFaceletResources. So it looks like this would constitute a change in behavior. Can we talk about this over email to see what else is there to be done? Thanks!

javaserverfaces commented 9 years ago

@edburns said: Here's what I've found about the now-failing test: I'm not sure it's valid (but I probably wrote it myself years and years ago).

The test does this:

Resource resource = handler.createResource("rootLibrary-duke.gif", "rootLibrary");

Where the resource is packaged in the war like this:

unzip -t test.war | grep rootLibrary testing: rootLibrary/ OK testing: rootLibrary/rootLibrary-duke.gif OK

As far as I can tell, this should not be found as a resource, but it was being found by the faceletResourceHelper. It should not have been found by the faceletResourceHelper.

I'm going to think about this some more. It's possible the test is invalid.

javaserverfaces commented 9 years ago

cappi said: I’ve investigated some more and created a new patch (attached), which also changes the failing test. The patch now makes sure, that facelets can be accessed by ResourceHandler.createResource(String resourceName). So instead of insisting that facelets can only be accessed by createViewResource, we can still call createResource.

javaserverfaces commented 9 years ago

@edburns said: I've investigated the history of the failing testcase, and indeed I had added it myself

r9752 | edburns | 2012-03-09 12:23:16 -0500 (Fri, 09 Mar 2012) | 24 lines

It was done as part of JAVASERVERFACES_SPEC_PUBLIC-719, filed by David Schneider for ADF.

javaserverfaces commented 9 years ago

@edburns said: Please consider this diff:

--- jsf-ri/src/main/java/com/sun/faces/application/resource/ResourceManager.java    (revision 9751)
+++ jsf-ri/src/main/java/com/sun/faces/application/resource/ResourceManager.java    (revision 9752)
@@ -354,8 +354,10 @@
      * specified <code>arguments</code>.
      * <p/>
      * <p> The lookup process will first search the file system of the web
-     * application.  If the library is not found, then it processed to
-     * searching the classpath.</p>
+     * application *within the resources directory*.  
+     * If the library is not found, then it processed to
+     * searching the classpath, if not found there, search from the webapp root
+     * *excluding* the resources directory.</p>

The last part, "search from the webapp root excluding the resources directory" is the part that Frank's initial patch invalidated. This part still needs to work. I'm trying Frank's 20141203-1115 iteration.

javaserverfaces commented 9 years ago

@edburns said: Hudson is clean.

javaserverfaces commented 9 years ago

File: 20141203-1601Z-i_moj_3428-test-build-mods.patch Attached By: @edburns

javaserverfaces commented 9 years ago

File: JAVASERVERFACES-3428-2.patch Attached By: cappi

javaserverfaces commented 10 years ago

File: JAVASERVERFACES-3428-master.zip Attached By: @manfredriem

javaserverfaces commented 10 years ago

Issue-Links: is cloned by JAVASERVERFACES-3677 JAVASERVERFACES-3782

javaserverfaces commented 10 years ago

Was assigned to cappi

javaserverfaces commented 7 years ago

This issue was imported from java.net JIRA JAVASERVERFACES-3428

javaserverfaces commented 9 years ago

Marked as fixed on Wednesday, January 7th 2015, 2:46:27 pm