ICEfaces
  1. ICEfaces
  2. ICE-8758

ice:outputStyle - Does not always work to deliver browser specific css files

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: EE-1.8.2.GA_P04, EE-3.0.0.GA_P01, 3.2
    • Fix Version/s: EE-3.3.0.GA, EE-1.8.2.GA_P07
    • Component/s: ICE-Components
    • Labels:
      None
    • Environment:
      All
    • Assignee Priority:
      P3
    • Salesforce Case Reference:

      Description


      In the "ice:outputStyle" taglib documentation it indicates that the filename contained
      in the 'href="file.css"' parameter will also attempt to deliver a browser-specific version of the CSS file
      if the browser-specific file is found, and the "file.css" if not.

      Icefaces uses the User-Agent header in the request to determine what browser-specific CSS file to deliver.

      Browser-specific CSS files look like "file_ie7.css" or "file_ie8.css".

      The way this delivery is accomplished in the original code is by appending a new "link" tag to the existing one, so there are then
      two links seen in the source, one for the original file and one for the new browser-specific (e.g. "..._ie8.css") one.

      In the example application we have two CSS files, and each of their browser-specific counterparts existing in the "css" directory:

      css/test1.css
      css/test1_ie7.css
      css/test1_ie8.css
      css/test2.css
      css/test2_ie7.css
      css/test2_ie8.css

      When "ice:outputStyle" tags are provided for both css/test1.css and css/test2.css, we should see four <link> tags like this:

      <link href="css/test1.css" rel="stylesheet" type="text/css" />
      <link href="css/test1_ie8.css" rel="stylesheet" title="_ie8" type="text/css" />
      <link href="css/test2.css" rel="stylesheet" type="text/css" />
      <link href="css/test2_ie8.css" rel="stylesheet" title="_ie8" type="text/css" />

      Unfortunately, sometimes this doesn't work.

      In particular, if an absolute path starting with "/" is used in the "href" attribute, or if a relative path starting with ".."
      is used, IceFaces OutputStyleRenderer code looks in the wrong place to determine if the browser-specific version of
      the file exists.
      1. Document6.txt
        5 kB
        yip.ng
      2. OutputStyleRenderer.java
        13 kB
        yip.ng
      1. screenshot-01.png
        218 kB
      2. screenshot-02.png
        120 kB
      3. screenshot-03.png
        88 kB
      4. screenshot-04.png
        96 kB
      5. screenshot-05.png
        117 kB
      6. screenshot-06.png
        126 kB
      7. screenshot-07.png
        119 kB
      8. screenshot-08.png
        278 kB
      9. screenshot-09.png
        137 kB
      10. screenshot-10.png
        151 kB

        Activity

        Hide
        Arran Mccullough added a comment - - edited

        Comment from the customer on their fix:
        To that end, we have created our own "medice:outputStyle" tag, which does provide the correct link tag in all valid circumstances.
        This tag is back by code that extends the original icefaces tag code.
        In the example application, we have four basic use cases:
        Absolute path, top directory
        Absolute path, child directory
        Relative path, top directory
        Relative path, child directory
        Each of these uses cases are covered in a separate page. Each one of the pages has one invokation of the "ice:outputStyle" and
        one invokation of our version of the "ice:outputStyle" tag - "medice:outputStyle".
        All through the examples, the "ice" version of the tag tries to deliver test1.css, and the "medice" version tries to deliver test2.css.
        In this back-to-back scenario, we can easily see that all valid paths work for the "medice" version, but some do not work for the
        original "ice" version of the tag. For example in the "relative/child" scenario for IE8, we should see:
        <link href="../css/test1.css" rel="stylesheet" type="text/css" />
        <link href="../css/test1_ie8.css" rel="stylesheet" title="_ie8" type="text/css" />
        <link href="../css/test2.css" rel="stylesheet" type="text/css" />
        <link href="../css/test2_ie8.css" rel="stylesheet" title="_ie8" type="text/css" />
        Instead, viewing the source, we see:
        <link href="../css/test1.css" rel="stylesheet" type="text/css" />
        <link href="../css/test2.css" rel="stylesheet" type="text/css" />
        <link href="../css/test2_ie8.css" rel="stylesheet" title="_ie8" type="text/css" />
        The "test1" css that is generated by the "ice" version of the tag, doesn't have the browser-specific link appended. That link tag is missing.
        This is incorrect. The "medice" version correctly delivers both in this case.
        The only use-case that works correctly is "relative/top".
        You want to look at our com.med.icefaces.style.OutputStyleRenderer.encodeEnd(...) method in the sample app.
        In it you will find that for the various use-cases, our custom extension of the Icefaces OutputStyleRenderer class searches
        for the real browser-specific file on the filesystem in the proper places based on the input "href" path supplied. In some cases,
        it will need to detect if the href starts with the context-root, and strip it off, so that the getRealPath can get the real path
        of the file, to determine if it really does exist.
        (Restricted to icesoft-internal-developers group)

        Show
        Arran Mccullough added a comment - - edited Comment from the customer on their fix: To that end, we have created our own "medice:outputStyle" tag, which does provide the correct link tag in all valid circumstances. This tag is back by code that extends the original icefaces tag code. In the example application, we have four basic use cases: Absolute path, top directory Absolute path, child directory Relative path, top directory Relative path, child directory Each of these uses cases are covered in a separate page. Each one of the pages has one invokation of the "ice:outputStyle" and one invokation of our version of the "ice:outputStyle" tag - "medice:outputStyle". All through the examples, the "ice" version of the tag tries to deliver test1.css, and the "medice" version tries to deliver test2.css. In this back-to-back scenario, we can easily see that all valid paths work for the "medice" version, but some do not work for the original "ice" version of the tag. For example in the "relative/child" scenario for IE8, we should see: <link href="../css/test1.css" rel="stylesheet" type="text/css" /> <link href="../css/test1_ie8.css" rel="stylesheet" title="_ie8" type="text/css" /> <link href="../css/test2.css" rel="stylesheet" type="text/css" /> <link href="../css/test2_ie8.css" rel="stylesheet" title="_ie8" type="text/css" /> Instead, viewing the source, we see: <link href="../css/test1.css" rel="stylesheet" type="text/css" /> <link href="../css/test2.css" rel="stylesheet" type="text/css" /> <link href="../css/test2_ie8.css" rel="stylesheet" title="_ie8" type="text/css" /> The "test1" css that is generated by the "ice" version of the tag, doesn't have the browser-specific link appended. That link tag is missing. This is incorrect. The "medice" version correctly delivers both in this case. The only use-case that works correctly is "relative/top". You want to look at our com.med.icefaces.style.OutputStyleRenderer.encodeEnd(...) method in the sample app. In it you will find that for the various use-cases, our custom extension of the Icefaces OutputStyleRenderer class searches for the real browser-specific file on the filesystem in the proper places based on the input "href" path supplied. In some cases, it will need to detect if the href starts with the context-root, and strip it off, so that the getRealPath can get the real path of the file, to determine if it really does exist. (Restricted to icesoft-internal-developers group)
        Hide
        Arran Mccullough added a comment - - edited

        Attached test case that shows the issue and the custom component code that works.

        Steps:

        • In an IE browser (IE9 in IE 8 Mode for example) open main.jsf.
        • Clicking on the various links will show different behavior with the render output for the outputStyle tag.
        • Looking at the rendered source code will show the issues.
        • The readme file will also display the information provided in this case.
        Show
        Arran Mccullough added a comment - - edited Attached test case that shows the issue and the custom component code that works. Steps: In an IE browser (IE9 in IE 8 Mode for example) open main.jsf. Clicking on the various links will show different behavior with the render output for the outputStyle tag. Looking at the rendered source code will show the issues. The readme file will also display the information provided in this case.
        Hide
        yip.ng added a comment - - edited

        (Can't even build test app. Can't copy and paste into 1.8 showcase either because test app. has its own taglib, tag handler, renderer, faces-config, etc. Therefore just use 1.8 showcase to simulate problem.)

        The "absolute/top" case is also actually working (without renderer modification), not just the "relative/top" case. See screenshot-01.png, screenshot-02.png.

        The "absolute/child" case is also working: screenshot-03.png.

        The "relative/child" case is also working: screenshot-04.png.

        So all cases seem to be working. Can't reproduce. Need to figure out how to build and run test app. first.

        Show
        yip.ng added a comment - - edited (Can't even build test app. Can't copy and paste into 1.8 showcase either because test app. has its own taglib, tag handler, renderer, faces-config, etc. Therefore just use 1.8 showcase to simulate problem.) The "absolute/top" case is also actually working (without renderer modification), not just the "relative/top" case. See screenshot-01.png , screenshot-02.png . The "absolute/child" case is also working: screenshot-03.png . The "relative/child" case is also working: screenshot-04.png . So all cases seem to be working. Can't reproduce. Need to figure out how to build and run test app. first.
        Hide
        Arran Mccullough added a comment -

        Looks like my runnable war files were removed in the JIRA recovery.

        Show
        Arran Mccullough added a comment - Looks like my runnable war files were removed in the JIRA recovery.
        Hide
        yip.ng added a comment -

        war files are useless to me. I need to change – build – run – debug – change ...... (in Intellij.)

        Show
        yip.ng added a comment - war files are useless to me. I need to change – build – run – debug – change ...... (in Intellij.)
        Hide
        yip.ng added a comment - - edited
        Show
        yip.ng added a comment - - edited Document6.txt .
        Hide
        yip.ng added a comment - - edited

        Done for 3.3: screenshot-05.png, screenshot-06.png, screenshot-07.png. Can't be done for 1.8 because getRealPath() is only available in JSF 2. We never used to check for existence of extra CSS file in 1.8: screenshot-09.png.

        User-contributed code is in OutputStyleRenderer.java. Bulk of essential code slightly refactored into method instead of coded inline, so that it is more readable and maintainable. Just minimal refactoring to avoid introducing bugs. By the same token, non-essential code changes were not adopted even though they look cleaner: e.g. screenshot-08.png.

        M: C:\svn\ossrepo\icefaces3\trunk\icefaces\compat\components\src\main\java\com\icesoft\faces\component\style\OutputStyleRenderer.java#34165

        Show
        yip.ng added a comment - - edited Done for 3.3: screenshot-05.png , screenshot-06.png , screenshot-07.png . Can't be done for 1.8 because getRealPath() is only available in JSF 2. We never used to check for existence of extra CSS file in 1.8: screenshot-09.png . User-contributed code is in OutputStyleRenderer.java . Bulk of essential code slightly refactored into method instead of coded inline, so that it is more readable and maintainable. Just minimal refactoring to avoid introducing bugs. By the same token, non-essential code changes were not adopted even though they look cleaner: e.g. screenshot-08.png . M: C:\svn\ossrepo\icefaces3\trunk\icefaces\compat\components\src\main\java\com\icesoft\faces\component\style\OutputStyleRenderer.java#34165
        Hide
        yip.ng added a comment - - edited

        [12:22:31 PM] Mark Collette: Yip: Ask Deryk about 1.8 ICEfaces utility method that might be equivalent to JSF 2 getRealPath

        Show
        yip.ng added a comment - - edited [12:22:31 PM] Mark Collette: Yip: Ask Deryk about 1.8 ICEfaces utility method that might be equivalent to JSF 2 getRealPath
        Hide
        yip.ng added a comment - - edited

        I believe com.icesoft.faces.util.CoreUtils has a couple of getRealPath methods that use reflection to call getRealPath safely in both portlet and servlet environments.

        Deryk

        Show
        yip.ng added a comment - - edited I believe com.icesoft.faces.util.CoreUtils has a couple of getRealPath methods that use reflection to call getRealPath safely in both portlet and servlet environments. Deryk
        Hide
        yip.ng added a comment - - edited

        Done for 1.8 as well using CoreUtils.getRealPath(): screenshot-10.png.

        M: C:\svn\ossrepo\icefaces\trunk\icefaces\component\src\com\icesoft\faces\component\style\OutputStyleRenderer.java#34220

        Show
        yip.ng added a comment - - edited Done for 1.8 as well using CoreUtils.getRealPath(): screenshot-10.png . M: C:\svn\ossrepo\icefaces\trunk\icefaces\component\src\com\icesoft\faces\component\style\OutputStyleRenderer.java#34220
        Hide
        Cruz Miraback added a comment -

        Confirmed fixed using ICEfaces EE 1.8.2.GA_P07 build2 in IE7/8. Attaching modified war file used for testing.

        Show
        Cruz Miraback added a comment - Confirmed fixed using ICEfaces EE 1.8.2.GA_P07 build2 in IE7/8. Attaching modified war file used for testing.

          People

          • Assignee:
            yip.ng
            Reporter:
            Arran Mccullough
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: