Wicket
  1. Wicket
  2. WICKET-4000

Header contributions order is not stable

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.5-RC6
    • Fix Version/s: 6.0.0-beta1
    • Component/s: wicket
    • Labels:
      None

      Description

      In the last RCs, I started to experience problems with the contributions order.
      For example, I add jQuery, and until 1.5RC5, it worked well, but now the call to the jQuery script has been moved to the bottom of the page head, and this disables all my other scripts that are expecting jQuery's $ to be defined.

      I attach a quickstart to demonstrate the problem.
      Maybe the order in the quickstart is not the expected one, but what it shows is that the order does not make real sense (at least to me) :
      In the quickstart, the wicket:head tag contributions are in the order 3 - 8 - 9 - 5, and the renderHead methods contributions are in the order 4 - 1 - 2 - 6 - 7.

        Issue Links

          Activity

          Hide
          Martin Grigorov added a comment -

          Running the quickstart with trunk and RC5.1 produces the same sequences: 3, 8, 9, 5, 4, 1, 2, 6, 7.
          And according to org.apache.wicket.markup.renderStrategy.ChildFirstHeaderRenderStrategy this is correct.

          Show
          Martin Grigorov added a comment - Running the quickstart with trunk and RC5.1 produces the same sequences: 3, 8, 9, 5, 4, 1, 2, 6, 7. And according to org.apache.wicket.markup.renderStrategy.ChildFirstHeaderRenderStrategy this is correct.
          Hide
          Martin Grigorov added a comment -

          Actually it seems in both versions wicket:head may play wrong.
          ChildFirstHeaderRenderStrategy says that components' contributions are rendered before page's contributions. So it seems script 5 must be rendered first, not fourth.

          Show
          Martin Grigorov added a comment - Actually it seems in both versions wicket:head may play wrong. ChildFirstHeaderRenderStrategy says that components' contributions are rendered before page's contributions. So it seems script 5 must be rendered first, not fourth.
          Hide
          Sylvain Vieujot added a comment -

          Ok. I understand the advantage of being able to have a child replace a parent contribution, but I wouldn't it be possible to keep an intuitive order, and to still have the last contribution replace the previous ones inplace ?

          Also here the logic is not followed completely as the Application contributions are rendered first anyway.

          Last, what I see is that the <wicket:head> is rendered before the code contributions of the renderHead method.
          I would suggest to invert this, as usually I guess you would use the renderHead to include more general code / libraries, and then the <wicket:head> to include more specific code, most likely using the libraries contributed by the renderHead method.

          Show
          Sylvain Vieujot added a comment - Ok. I understand the advantage of being able to have a child replace a parent contribution, but I wouldn't it be possible to keep an intuitive order, and to still have the last contribution replace the previous ones inplace ? Also here the logic is not followed completely as the Application contributions are rendered first anyway. Last, what I see is that the <wicket:head> is rendered before the code contributions of the renderHead method. I would suggest to invert this, as usually I guess you would use the renderHead to include more general code / libraries, and then the <wicket:head> to include more specific code, most likely using the libraries contributed by the renderHead method.
          Hide
          Martin Grigorov added a comment -

          See WICKET-3761 for the reasons.

          Show
          Martin Grigorov added a comment - See WICKET-3761 for the reasons.
          Hide
          Sylvain Vieujot added a comment -

          It looks to me that the above suggestion (contribution order identical to wicket 1.4, but still allowed contributions code override from children) can be implemented by adding a kind of buffer in HeaderResponse, and flushing it once all the headerContributions are done.

          It would seem to me a much more user friendly approach, whilst keeping the advantage of being able to override previous contributions.
          At least, when I look at my webapps, the current wicket 1.5 contributions order gives me headaches.

          What do you think ?

          Show
          Sylvain Vieujot added a comment - It looks to me that the above suggestion (contribution order identical to wicket 1.4, but still allowed contributions code override from children) can be implemented by adding a kind of buffer in HeaderResponse, and flushing it once all the headerContributions are done. It would seem to me a much more user friendly approach, whilst keeping the advantage of being able to override previous contributions. At least, when I look at my webapps, the current wicket 1.5 contributions order gives me headaches. What do you think ?
          Hide
          Martin Grigorov added a comment -

          This buffering is possible with ResponseHeaderDecorator. See http://www.wicket-library.com/wicket-examples/resourceaggregation for example.
          But I think this example doesn't use wicket:head.

          Show
          Martin Grigorov added a comment - This buffering is possible with ResponseHeaderDecorator. See http://www.wicket-library.com/wicket-examples/resourceaggregation for example. But I think this example doesn't use wicket:head.
          Hide
          Martin Grigorov added a comment -

          What we are going to do here ?
          The order is the same as in previous RCs of 1.5 (after WICKET-3761).
          Reordering is possible with custom IHeaderResponseDecorator. See org.apache.wicket.resource.aggregation.AbstractResourceAggregatingHeaderResponse.

          Show
          Martin Grigorov added a comment - What we are going to do here ? The order is the same as in previous RCs of 1.5 (after WICKET-3761 ). Reordering is possible with custom IHeaderResponseDecorator. See org.apache.wicket.resource.aggregation.AbstractResourceAggregatingHeaderResponse.
          Hide
          Igor Vaynberg added a comment -

          imho its too late to tweak this. save it for wicket.next

          Show
          Igor Vaynberg added a comment - imho its too late to tweak this. save it for wicket.next
          Hide
          Sylvain Vieujot added a comment -

          I don't think we should postpone this for 1.6.
          Indeed, 1.5 is changing the default order, and I think this might lead to some breaks in the webapps that are hard to track.
          For example, when it breaks the javascript ordering, no compilation warning is going to alert the developers, and only a careful integration testing can detect this. I think webapps using wiquery might suffer from this new ordering (my webapps at least failed).

          Furthermore, changing the order in 1.5 whilst knowing that we might revert back to the 1.4 order in 1.6 does not seem right to me.
          I would suggest trying to fix this asap, and I am ready to start a patch if you think I am on the right track (It would be my first wicket hack, so don't expect miracles ...).

          Show
          Sylvain Vieujot added a comment - I don't think we should postpone this for 1.6. Indeed, 1.5 is changing the default order, and I think this might lead to some breaks in the webapps that are hard to track. For example, when it breaks the javascript ordering, no compilation warning is going to alert the developers, and only a careful integration testing can detect this. I think webapps using wiquery might suffer from this new ordering (my webapps at least failed). Furthermore, changing the order in 1.5 whilst knowing that we might revert back to the 1.4 order in 1.6 does not seem right to me. I would suggest trying to fix this asap, and I am ready to start a patch if you think I am on the right track (It would be my first wicket hack, so don't expect miracles ...).
          Hide
          Martin Grigorov added a comment -

          The order from 1.4 is still possible. See org.apache.wicket.markup.renderStrategy.AbstractHeaderRenderStrategy.get().
          For Wicket.next we will consider making it easier for the application developer to specify the contribution order. But I can see how this can be problematic for component libraries.

          With the current approach (child first) the components contribute their js/css and the page can override them. This is also what you are trying to so, right ?

          Show
          Martin Grigorov added a comment - The order from 1.4 is still possible. See org.apache.wicket.markup.renderStrategy.AbstractHeaderRenderStrategy.get(). For Wicket.next we will consider making it easier for the application developer to specify the contribution order. But I can see how this can be problematic for component libraries. With the current approach (child first) the components contribute their js/css and the page can override them. This is also what you are trying to so, right ?
          Hide
          Sylvain Vieujot added a comment -

          In fact, I am not too much concerned about the override capability. It might be an advantage, but I do not use it or see any use case for this in my applications.

          Here are 2 simple scenari that fail with me and that would seem quite an intuitive way to use wicket :

          1) You use jQuery in part of the public pages of your website. so it makes sense to add the jQuery contribution in the PublicBasePage.
          But as the PublicBasePage contributions come after the children ones, any Panel using jQuery will fail as jQuery will not be loaded yet.

          2) Again you want to use jQuery in a page. It makes more sense to put the contribution in the renderHead than in the wicket:head as it is more type safe.
          Also, in renderHead, you can use Wiquery's response.renderJavaScriptReference( CoreJavaScriptResourceReference.get() );
          In the page, you want to add a custom jquery one liner. It is then much easier to put this one liner in the wicket:head as it will be in the same html, and much less verbose than doing this in the renderHead.
          But again this fails, as currently the default is to have first the wicket:head contributions, and later the renderHead.

          I understood that you can re-write your own strategy (even if I am not sure that the wicket:head / renderHead order can be altered), but I am quite reluctant to do so, as this means you might have a component library that can work only with some specific webapps.
          Also, those problems are quite nasty, as you do not detect them on compilation.

          That's why I would suggest :
          1) To have 2 renderHead methods renderHeadBeforeMarkup & renderHeadAfterMarkup (or whatever better name).
          At least this make the component libraries completely safe and the coding intuitive.

          2) To use the former wicket 1.4 default order which is much more intuitive (and which will have the advantage to limit the breakage in the legacy applications).
          This is also in line with the java code where the base class constructor is always called before the sub class one (hence the order assumed in my quickstart which would be the natural one for someone not delving into wicket's code).
          The implementation would require implementing a buffer in the HeaderResponse to allow the override of parent contributions by the children (if this is a useful feature - frankly I am not sure).
          I guess that would also require an additional method (like header.flush) that would be called in the renderCycle.

          With those changes, it would also remove the need to have a configurable renderStrategy.

          Show
          Sylvain Vieujot added a comment - In fact, I am not too much concerned about the override capability. It might be an advantage, but I do not use it or see any use case for this in my applications. Here are 2 simple scenari that fail with me and that would seem quite an intuitive way to use wicket : 1) You use jQuery in part of the public pages of your website. so it makes sense to add the jQuery contribution in the PublicBasePage. But as the PublicBasePage contributions come after the children ones, any Panel using jQuery will fail as jQuery will not be loaded yet. 2) Again you want to use jQuery in a page. It makes more sense to put the contribution in the renderHead than in the wicket:head as it is more type safe. Also, in renderHead, you can use Wiquery's response.renderJavaScriptReference( CoreJavaScriptResourceReference.get() ); In the page, you want to add a custom jquery one liner. It is then much easier to put this one liner in the wicket:head as it will be in the same html, and much less verbose than doing this in the renderHead. But again this fails, as currently the default is to have first the wicket:head contributions, and later the renderHead. I understood that you can re-write your own strategy (even if I am not sure that the wicket:head / renderHead order can be altered), but I am quite reluctant to do so, as this means you might have a component library that can work only with some specific webapps. Also, those problems are quite nasty, as you do not detect them on compilation. That's why I would suggest : 1) To have 2 renderHead methods renderHeadBeforeMarkup & renderHeadAfterMarkup (or whatever better name). At least this make the component libraries completely safe and the coding intuitive. 2) To use the former wicket 1.4 default order which is much more intuitive (and which will have the advantage to limit the breakage in the legacy applications). This is also in line with the java code where the base class constructor is always called before the sub class one (hence the order assumed in my quickstart which would be the natural one for someone not delving into wicket's code). The implementation would require implementing a buffer in the HeaderResponse to allow the override of parent contributions by the children (if this is a useful feature - frankly I am not sure). I guess that would also require an additional method (like header.flush) that would be called in the renderCycle. With those changes, it would also remove the need to have a configurable renderStrategy.
          Hide
          Martin Grigorov added a comment -

          I see you are confused.

          If a component depends on JS library then the component should add a reference for it. It should not expect that the page that uses this component will provide it. The component can implement means to switch off the contribution or to override the ResourceReference if it wants, so it will be possible to use different version of the JS library.

          The page contribution should come after the components so that even if the component library doesn't provide ways to switch off/replace the contribution then the page can still override it. This is more valid for CSS contributions and less for JS ones.
          As final resort Wicket provides IHeaderResponseDecorator hook so you are able to suppress any ResourceReference you want.

          Additionally there are Application scoped IHeaderContributor's which will render their CSS/JS for all pages and always before the pages/components.

          The only difference between 1.4 and 1.5 if that components now contribute before the page. That's all. There are no differences in wicket:head vs. renderHead() order.
          Additionally there is no sense to use wicket:head in the base Page. It is for Panels and Borders and inherited Pages. In the base page you can (and should) use <head>.

          About Java constructors: it is the same in Wicket.
          wicket:head order in the quickstart: 3 (BasePage.html) and then 8, 9 (HomePage.html)
          renderHead order: 1, 2 (BasePage.java) and then 6, 7 (HomePage.java)

          As I said earlier the only one that looks out of order is the Panel contribution. I expect 5 (Header.html) and 4 (Header.java) to be the first two in the line, not in the middle as now.

          1.5.0 is already voted so it is not possible to shuffle them more for 1.5.x thus the ticket is moved to 1.6.x.

          Show
          Martin Grigorov added a comment - I see you are confused. If a component depends on JS library then the component should add a reference for it. It should not expect that the page that uses this component will provide it. The component can implement means to switch off the contribution or to override the ResourceReference if it wants, so it will be possible to use different version of the JS library. The page contribution should come after the components so that even if the component library doesn't provide ways to switch off/replace the contribution then the page can still override it. This is more valid for CSS contributions and less for JS ones. As final resort Wicket provides IHeaderResponseDecorator hook so you are able to suppress any ResourceReference you want. Additionally there are Application scoped IHeaderContributor's which will render their CSS/JS for all pages and always before the pages/components. The only difference between 1.4 and 1.5 if that components now contribute before the page. That's all. There are no differences in wicket:head vs. renderHead() order. Additionally there is no sense to use wicket:head in the base Page. It is for Panels and Borders and inherited Pages. In the base page you can (and should) use <head>. About Java constructors: it is the same in Wicket. wicket:head order in the quickstart: 3 (BasePage.html) and then 8, 9 (HomePage.html) renderHead order: 1, 2 (BasePage.java) and then 6, 7 (HomePage.java) As I said earlier the only one that looks out of order is the Panel contribution. I expect 5 (Header.html) and 4 (Header.java) to be the first two in the line, not in the middle as now. 1.5.0 is already voted so it is not possible to shuffle them more for 1.5.x thus the ticket is moved to 1.6.x.
          Hide
          Sylvain Vieujot added a comment -

          Then I think my problem boils down to WICKET-3761 .
          I want to contribute jQuery via a renderHead method, or via the Applications's HeaderContributors, and currently this means that the jquery library will never be available to any code in a wicket:head.

          This is the same as Igor mentioned here : https://issues.apache.org/jira/browse/WICKET-3761?focusedCommentId=13044321&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13044321

          Do I have a solution for this ?
          The only one I see now is to avoid using wicket:head, and use a <script> tag in the <body> instead.

          Thank you.

          Show
          Sylvain Vieujot added a comment - Then I think my problem boils down to WICKET-3761 . I want to contribute jQuery via a renderHead method, or via the Applications's HeaderContributors, and currently this means that the jquery library will never be available to any code in a wicket:head. This is the same as Igor mentioned here : https://issues.apache.org/jira/browse/WICKET-3761?focusedCommentId=13044321&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13044321 Do I have a solution for this ? The only one I see now is to avoid using wicket:head, and use a <script> tag in the <body> instead. Thank you.
          Hide
          Igor Vaynberg added a comment -

          we should consider reverting WICKET-3761

          Show
          Igor Vaynberg added a comment - we should consider reverting WICKET-3761
          Hide
          Sylvain Vieujot added a comment -

          This would be very helpful !

          Show
          Sylvain Vieujot added a comment - This would be very helpful !
          Hide
          Sylvain Vieujot added a comment -

          For information, there was another problem linked to WiQuery (Maybe this issue : http://code.google.com/p/wiquery/issues/detail?id=203 ), and this was inverting the order of some renderHead contributions ... making the all issue even stranger.
          wiquery-jquery-ui 1.5-M4 still has the issue, but switching to wiquery-jquery-ui 1.5-SNAPSHOT resolved this specific issue.

          Show
          Sylvain Vieujot added a comment - For information, there was another problem linked to WiQuery (Maybe this issue : http://code.google.com/p/wiquery/issues/detail?id=203 ), and this was inverting the order of some renderHead contributions ... making the all issue even stranger. wiquery-jquery-ui 1.5-M4 still has the issue, but switching to wiquery-jquery-ui 1.5-SNAPSHOT resolved this specific issue.
          Hide
          Emond Papegaaij added a comment -

          Reverting WICKET-3761 just inverts the arbitrarily chosen order and breaks applications that depend on this order. The major issue is that wicket:head is rendered by a different code path than other header contributions. wicket:head should be rendered by IHeaderResponse, making it possible to change the order using rendering strategies or a HeaderResponseDecorator. The default order should stay as it is right now as both orders are equally valid. I'm willing to take this issue after I'm finished with WICKET-4273.

          Show
          Emond Papegaaij added a comment - Reverting WICKET-3761 just inverts the arbitrarily chosen order and breaks applications that depend on this order. The major issue is that wicket:head is rendered by a different code path than other header contributions. wicket:head should be rendered by IHeaderResponse, making it possible to change the order using rendering strategies or a HeaderResponseDecorator. The default order should stay as it is right now as both orders are equally valid. I'm willing to take this issue after I'm finished with WICKET-4273 .
          Hide
          Emond Papegaaij added a comment -

          With this patch, all header rendering is done by IHeaderResponse from within the HeaderRenderStrategy. Some tests are failing now, because the order changed, but I think the order defined in these testcases is wrong.

          Show
          Emond Papegaaij added a comment - With this patch, all header rendering is done by IHeaderResponse from within the HeaderRenderStrategy. Some tests are failing now, because the order changed, but I think the order defined in these testcases is wrong.
          Hide
          Emond Papegaaij added a comment -

          Final version of the patch

          Show
          Emond Papegaaij added a comment - Final version of the patch
          Hide
          Martin Grigorov added a comment -

          Fixed in 6.0.x.
          The only difference is that wicket:head contributes before same component's #renderHead(). This way the dynamic code can override the static code

          Show
          Martin Grigorov added a comment - Fixed in 6.0.x. The only difference is that wicket:head contributes before same component's #renderHead(). This way the dynamic code can override the static code

            People

            • Assignee:
              Martin Grigorov
              Reporter:
              Sylvain Vieujot
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development