Wicket
  1. Wicket
  2. WICKET-4247

HeaderResponseContainerFilteringHeaderResponse does not override enough methods

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.5.3
    • Fix Version/s: 1.5.4
    • Component/s: wicket
    • Labels:
      None

      Description

      Seen on Wicket 1.5.3 and I think it is still there in SVN.

      Major because users might be trapped thinking that their Javascript is at the bottom but it is not, having weird consequences.

      Attached is a quickstart. It sets up a header response decorator with a header response sending javascript to the bottom which is optionally wrapped by a simple implementation of an aggregating header response. The wrapper though uses a full-arguments method to render javascript which is not fetched by the other response header. So the javascript ends up in the html head.

      It should suffice to override the remaining four (js/css, resource ref/url) render methods to fix this.

        Issue Links

          Activity

          Show
          Emond Papegaaij added a comment - You are right. I've committed your patch: http://git-wip-us.apache.org/repos/asf?p=wicket.git;a=commit;h=56ab23584fcb1c3949e69255078239c85dacea69
          Hide
          Andreas Köhler added a comment -

          I think the patch had a typo, at least my app stopped working...
          Please consider the new patch which sits on top of the other.

          Show
          Andreas Köhler added a comment - I think the patch had a typo, at least my app stopped working... Please consider the new patch which sits on top of the other.
          Hide
          Andreas Köhler added a comment -

          Oh, I missed that. Great decision, btw :-D

          Show
          Andreas Köhler added a comment - Oh, I missed that. Great decision, btw :-D
          Hide
          Martin Grigorov added a comment -
          Show
          Martin Grigorov added a comment - I did: http://git-wip-us.apache.org/repos/asf?p=wicket.git;a=commitdiff;h=4e760f5a I switched to Git recently.
          Hide
          Andreas Köhler added a comment -

          Martin, did you really apply the latest patch? I cannot find it.
          Kudos to Emond!

          Show
          Andreas Köhler added a comment - Martin, did you really apply the latest patch? I cannot find it. Kudos to Emond!
          Hide
          Emond Papegaaij added a comment -

          Fixes the issue in 1.5 by overriding the last 2 methods.

          Show
          Emond Papegaaij added a comment - Fixes the issue in 1.5 by overriding the last 2 methods.
          Hide
          Andreas Köhler added a comment -

          Sure, just give me a few days to find some spare time

          Show
          Andreas Köhler added a comment - Sure, just give me a few days to find some spare time
          Hide
          Martin Grigorov added a comment -

          Applying a patch is easy
          Attach it and I'll commit it.

          Show
          Martin Grigorov added a comment - Applying a patch is easy Attach it and I'll commit it.
          Hide
          Andreas Köhler added a comment -

          It is no stopper for me, but only because I work around and do not use the missing functions.
          +1 for Emond's comment.

          Show
          Andreas Köhler added a comment - It is no stopper for me, but only because I work around and do not use the missing functions. +1 for Emond's comment.
          Hide
          Emond Papegaaij added a comment -

          Perhaps for 1.5, it's as easy as adding the last few methods? New methods will not get added in 1.5, as that's an API break.

          Show
          Emond Papegaaij added a comment - Perhaps for 1.5, it's as easy as adding the last few methods? New methods will not get added in 1.5, as that's an API break.
          Hide
          Martin Grigorov added a comment -

          @Andreas: do you need this fix in 1.5 ?
          These classes are reworked in 6.0 and I'd like to save myself some time if this ticket is only for consistency and completeness and it is not really a stopper for you.

          Show
          Martin Grigorov added a comment - @Andreas: do you need this fix in 1.5 ? These classes are reworked in 6.0 and I'd like to save myself some time if this ticket is only for consistency and completeness and it is not really a stopper for you.
          Hide
          Emond Papegaaij added a comment -

          In the branch on github at https://github.com/papegaaij/wicket/commits/wicket+wiquery IHeaderResponse has been rewritten, including the filters, which fixes this issue.

          Show
          Emond Papegaaij added a comment - In the branch on github at https://github.com/papegaaij/wicket/commits/wicket+wiquery IHeaderResponse has been rewritten, including the filters, which fixes this issue.
          Hide
          Martin Grigorov added a comment -

          It is fragile
          But this is just because not that many people use these classes...

          I'd like to rework IHeaderResponse to have just: renderText(), renderCss(CssAttributes) and renderJavaScript(JavaScriptAttributes). These XyzAttributes will have all specifics for <link/> and <script/>.
          Not sure whether this will be part of Wicket 6.0 but it should be soon because more people like you will start using this fragile classes

          Show
          Martin Grigorov added a comment - It is fragile But this is just because not that many people use these classes... I'd like to rework IHeaderResponse to have just: renderText(), renderCss(CssAttributes) and renderJavaScript(JavaScriptAttributes). These XyzAttributes will have all specifics for <link/> and <script/>. Not sure whether this will be part of Wicket 6.0 but it should be soon because more people like you will start using this fragile classes
          Hide
          Andreas Köhler added a comment -

          I think I was wrong about "It should suffice to override the remaining four (js/css, resource ref/url) render methods to fix this. ".

          There are still two methods missing. This looks fragile to me

          So, what shall I do about?
          #renderJavaScriptReference(String, String, boolean)
          #renderJavaScriptReference(String, String, boolean, String)

          Show
          Andreas Köhler added a comment - I think I was wrong about "It should suffice to override the remaining four (js/css, resource ref/url) render methods to fix this. ". There are still two methods missing. This looks fragile to me So, what shall I do about? #renderJavaScriptReference(String, String, boolean) #renderJavaScriptReference(String, String, boolean, String)
          Hide
          Andreas Köhler added a comment -

          Quickstart
          Watch WicketApplication.PROXY_HEADER_RESPONSE

          Show
          Andreas Köhler added a comment - Quickstart Watch WicketApplication.PROXY_HEADER_RESPONSE

            People

            • Assignee:
              Martin Grigorov
              Reporter:
              Andreas Köhler
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development