Wicket
  1. Wicket
  2. WICKET-4894

Internet Explorer fails fails to properly include conditional stylesheet links added via AjaxRequestTarget

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.5.4
    • Fix Version/s: 6.4.0, 1.5.10
    • Component/s: wicket
    • Labels:
      None

      Description

      CSS references added like this

      @Override
      public void renderHead( IHeaderResponse response )

      { super.renderHead( response ); response.renderCSSReference( new PackageResourceReference( getClass(), "TestLabel.css" )); response.renderCSSReference( new PackageResourceReference( getClass(), "TestLabel-conditional.css" ), null, null, "gte IE 8" ); }

      will not be taken into account by Internet Explorer (tested with 8 and 9) at when rendering the document.
      I stepped though wicket-ajax.js and saw that the function added in Wicket.Head.Contributor.processComment will actually be executed and a new child node will be appended to the head node of the document. Unfortunately it seems IE disregards conditional comments added in this way.

      I encountered this using Wicket 1.5.4.
      I'll upload a quickstart.

      1. quickstart.zip
        22 kB
        Jonas Pohlandt
      2. HeaderResponse.java.diff
        0.3 kB
        Jonas Pohlandt

        Activity

        Hide
        Jonas Pohlandt added a comment -

        Quickstart

        Show
        Jonas Pohlandt added a comment - Quickstart
        Hide
        Martin Grigorov added a comment -

        Hi,

        I've experimented with this and it seems IE CSS engine doesn't check for dynamically injected links in conditional comments.
        One solution is wicket-ajax.js to parse the condition before injectiing the element in the <head> and inject it only when necessary. But this doesn't look nice at all.

        A much better solution is to check this at the server side:
        WebClientInfo clientInfo = (WebClientInfo) getSession().getClientInfo();
        ClientProperties properties = clientInfo.getProperties();
        if (properties.isBrowserInternetExplorer() && properties.getBrowserVersionMajor() >= 8)

        { response.renderCSSReference( new PackageResourceReference( getClass(), "TestLabel-conditional.css" )); }

        Nice and clean.

        Show
        Martin Grigorov added a comment - Hi, I've experimented with this and it seems IE CSS engine doesn't check for dynamically injected links in conditional comments. One solution is wicket-ajax.js to parse the condition before injectiing the element in the <head> and inject it only when necessary. But this doesn't look nice at all. A much better solution is to check this at the server side: WebClientInfo clientInfo = (WebClientInfo) getSession().getClientInfo(); ClientProperties properties = clientInfo.getProperties(); if (properties.isBrowserInternetExplorer() && properties.getBrowserVersionMajor() >= 8) { response.renderCSSReference( new PackageResourceReference( getClass(), "TestLabel-conditional.css" )); } Nice and clean.
        Hide
        Jonas Pohlandt added a comment - - edited

        Hi Martin,

        thanks for the quick reply and the solution. It took me a while to figure out the cause of this (and might take other people a while as well), so I suppose a warning when using the response.renderCSSReference overload with the "condition" parameter during an ajax request would be very helpful. Do you accept patches or pull requests on GitHub?

        Another thing: is getSession().getClientInfo() going to work during an ajax request if called for the first time in the lifetime of the session?

        Cheers
        Jonas

        Show
        Jonas Pohlandt added a comment - - edited Hi Martin, thanks for the quick reply and the solution. It took me a while to figure out the cause of this (and might take other people a while as well), so I suppose a warning when using the response.renderCSSReference overload with the "condition" parameter during an ajax request would be very helpful. Do you accept patches or pull requests on GitHub? Another thing: is getSession().getClientInfo() going to work during an ajax request if called for the first time in the lifetime of the session? Cheers Jonas
        Hide
        Martin Grigorov added a comment -

        Patches are always welcome!
        We prefer patch files attached in the ticket though. By attaching it you grant Apache the permissions to use your code.

        Show
        Martin Grigorov added a comment - Patches are always welcome! We prefer patch files attached in the ticket though. By attaching it you grant Apache the permissions to use your code.
        Hide
        Martin Grigorov added a comment -

        Q: Another thing: is getSession().getClientInfo() going to work during an ajax request if called for the first time in the lifetime of the session?
        A: this logic uses the User-Agent request header so it should work.

        Show
        Martin Grigorov added a comment - Q: Another thing: is getSession().getClientInfo() going to work during an ajax request if called for the first time in the lifetime of the session? A: this logic uses the User-Agent request header so it should work.
        Hide
        Jonas Pohlandt added a comment -

        diff for 1.5.9 HeaderResponse (do you even still accept patches for 1.5?). Have not used 6.x yet...

        Show
        Jonas Pohlandt added a comment - diff for 1.5.9 HeaderResponse (do you even still accept patches for 1.5?). Have not used 6.x yet...
        Hide
        Martin Grigorov added a comment -

        The log warning + updates to the javadoc has been added for 1.5.10 and 6.4.0.

        Show
        Martin Grigorov added a comment - The log warning + updates to the javadoc has been added for 1.5.10 and 6.4.0.
        Hide
        David vandendriessche added a comment - - edited

        The log warning always appears even if you use Chrome. Is there a way to change this? Or add it to a higher loglevel since it only gives info. There's also no info about the warning in 6.x.x docs http://ci.apache.org/projects/wicket/apidocs/6.0.x/org/apache/wicket/markup/head/IHeaderResponse.html

        Show
        David vandendriessche added a comment - - edited The log warning always appears even if you use Chrome. Is there a way to change this? Or add it to a higher loglevel since it only gives info. There's also no info about the warning in 6.x.x docs http://ci.apache.org/projects/wicket/apidocs/6.0.x/org/apache/wicket/markup/head/IHeaderResponse.html
        Hide
        Martin Grigorov added a comment -

        The log statement tells you that you try to make the user experience for IE users better but IE wont appreciate it. So you can either rework it as suggested above or remove it completely.

        We can make the log INFO or even DEBUG but then more people wont see it at all and will file tickets here when they realize that their code doesn't work for IE at all.
        You can always set ERROR level as minimum for this class in your logging configuration.

        Show
        Martin Grigorov added a comment - The log statement tells you that you try to make the user experience for IE users better but IE wont appreciate it. So you can either rework it as suggested above or remove it completely. We can make the log INFO or even DEBUG but then more people wont see it at all and will file tickets here when they realize that their code doesn't work for IE at all. You can always set ERROR level as minimum for this class in your logging configuration.

          People

          • Assignee:
            Martin Grigorov
            Reporter:
            Jonas Pohlandt
          • Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development