Wicket
  1. Wicket
  2. WICKET-4766

multiple <style> tags in header are rendered incorrectly

    Details

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

      Description

      I created a small quickstart.
      The BasePage has some multiple <style> tags. Only he first one is rendered correctly, all following render the tag body only, the surrounding <style></style> is missing.

      1. myproject.zip
        21 kB
        Marieke Vandamme
      2. WICKET-4766-testcase.patch
        3 kB
        Sven Meier

        Activity

        Emond Papegaaij made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Fix Version/s 6.1.0 [ 12322957 ]
        Resolution Fixed [ 1 ]
        Hide
        Emond Papegaaij added a comment -

        I'm closing this ticket, because this issue is now fixed. If anyone finds the comments about StringHeaderItem.equals or the filtering of duplicate references in head-sections still relevant, I think opening a new ticket would be best.

        Show
        Emond Papegaaij added a comment - I'm closing this ticket, because this issue is now fixed. If anyone finds the comments about StringHeaderItem.equals or the filtering of duplicate references in head-sections still relevant, I think opening a new ticket would be best.
        Hide
        Emond Papegaaij added a comment -

        I've also found a problem in your proposed testcase. By including a.js in both the panel and the page, via head, Wicket will not be able to filter duplicates, therefore a.js will be included twice in the output. Your testcase includes a.js only once in the output, is this intentional? If so, how do you suggest Wicket filters these duplicate references?

        Show
        Emond Papegaaij added a comment - I've also found a problem in your proposed testcase. By including a.js in both the panel and the page, via head, Wicket will not be able to filter duplicates, therefore a.js will be included twice in the output. Your testcase includes a.js only once in the output, is this intentional? If so, how do you suggest Wicket filters these duplicate references?
        Hide
        Emond Papegaaij added a comment -

        I think this problem is manifold. First of all, I've discovered that HtmlHeaderContainer renders a StringHeaderItem at line 230. This should be a PageHeaderItem. Secondly, I suspect that StringHeaderItem and PageHeaderItem should not override equals and hashcode. The argumentation for this would be that these are free-form and therefore no assumptions can be made about the semantics of the contents. With duplicate JS or CSS references, it is clear that duplicates should not be rendered, but as can be seen in this case, duplication has no meaning for StringHeaderItem. Finally, it seems that HtmlHeaderContainer.renderNext is not needed at all. This is the method that splits the headers into many small parts.

        The first observation should be fixed, because it is a bug.

        Removing the method that splits the header items, fixes this ticket. It breaks 2 others, but only because some whitespace in the header changes.

        Changing the second observation fixes this bug, but breaks a testcase: HeaderSectionTest.renderHomePage_20(). This testcase renders a StringHeaderItem in a component that is added twice. Changing the equals causes the item to be rendered twice. Perhaps StringHeaderItem should be rendered once per component class, but the HeaderItem does not know the class of the component. In all, changing this right now probably breaks applications, so we should probably opt for the other solution and leave this as is.

        Show
        Emond Papegaaij added a comment - I think this problem is manifold. First of all, I've discovered that HtmlHeaderContainer renders a StringHeaderItem at line 230. This should be a PageHeaderItem. Secondly, I suspect that StringHeaderItem and PageHeaderItem should not override equals and hashcode. The argumentation for this would be that these are free-form and therefore no assumptions can be made about the semantics of the contents. With duplicate JS or CSS references, it is clear that duplicates should not be rendered, but as can be seen in this case, duplication has no meaning for StringHeaderItem. Finally, it seems that HtmlHeaderContainer.renderNext is not needed at all. This is the method that splits the headers into many small parts. The first observation should be fixed, because it is a bug. Removing the method that splits the header items, fixes this ticket. It breaks 2 others, but only because some whitespace in the header changes. Changing the second observation fixes this bug, but breaks a testcase: HeaderSectionTest.renderHomePage_20(). This testcase renders a StringHeaderItem in a component that is added twice. Changing the equals causes the item to be rendered twice. Perhaps StringHeaderItem should be rendered once per component class, but the HeaderItem does not know the class of the component. In all, changing this right now probably breaks applications, so we should probably opt for the other solution and leave this as is.
        Hide
        Sven Meier added a comment -

        Sorry that I don't have a solution currently. I'll have to dig deeper into the new HeaderItem stuff to be able to sort out the problem. Perhaps Edmond can spot the problem right away.

        I don't understand why the header in the page's markup should be handled differently than the header in panels. But this is the case currently:

        • the page's header is sliced into too much HeaderItems
        • the panel's header is handled as a block, thus possibly resulting in duplicated js contribution.
        Show
        Sven Meier added a comment - Sorry that I don't have a solution currently. I'll have to dig deeper into the new HeaderItem stuff to be able to sort out the problem. Perhaps Edmond can spot the problem right away. I don't understand why the header in the page's markup should be handled differently than the header in panels. But this is the case currently: the page's header is sliced into too much HeaderItems the panel's header is handled as a block, thus possibly resulting in duplicated js contribution.
        Sven Meier made changes -
        Attachment WICKET-4766-testcase.patch [ 12546061 ]
        Hide
        Sven Meier added a comment -

        changed markup to show the problem and one additional one:

        • second <style> tags in page markup is rendered without tags
        • js header contribution from panel duplicates js header contribution from page
        Show
        Sven Meier added a comment - changed markup to show the problem and one additional one: second <style> tags in page markup is rendered without tags js header contribution from panel duplicates js header contribution from page
        Sven Meier made changes -
        Attachment WICKET-4766-testcase.patch [ 12546053 ]
        Martin Grigorov made changes -
        Assignee Emond Papegaaij [ papegaaij ]
        Sven Meier made changes -
        Attachment WICKET-4766-testcase.patch [ 12546053 ]
        Hide
        Sven Meier added a comment -

        Modified test classes for HeaderResponseTest showing the problem.

        Show
        Sven Meier added a comment - Modified test classes for HeaderResponseTest showing the problem.
        Sven Meier made changes -
        Summary Merging html markup generates incorrect html multiple <style> tags in header are rendered incorrectly
        Description I created a small quickstart.
        My HomePage extends my BasePage.
        HomePage has wicket:extend tag and my BasePage wicket:child tag.
        The BasePage has some js imports, which render incorrect when my HomePage is called.
        I created a small quickstart.
        The BasePage has some multiple <style> tags. Only he first one is rendered correctly, all following render the tag body only, the surrounding <style></style> is missing.
        Hide
        Marieke Vandamme added a comment -

        Hi Sven, Is this something you can fix or can you assign somebody else to this issue?

        The problem is that I can't use a workaround. The template is generated by our drupal website,
        and we don't have any control over it.

        Show
        Marieke Vandamme added a comment - Hi Sven, Is this something you can fix or can you assign somebody else to this issue? The problem is that I can't use a workaround. The template is generated by our drupal website, and we don't have any control over it.
        Hide
        Sven Meier added a comment -

        Sorry, I've misread the html output.

        The problem is that HtmlHeaderContainer#renderNext() splits <style>, body and </style> each in its own PageHeaderItem. Since duplicate header items are filtered from output, successive style tags are missing from the generated HTML.

        As a workaround I'd suggest using a single <style> tag or to wrap the <style> tags in a <wicket:link> tag.

        Show
        Sven Meier added a comment - Sorry, I've misread the html output. The problem is that HtmlHeaderContainer#renderNext() splits <style>, body and </style> each in its own PageHeaderItem. Since duplicate header items are filtered from output, successive style tags are missing from the generated HTML. As a workaround I'd suggest using a single <style> tag or to wrap the <style> tags in a <wicket:link> tag.
        Hide
        Marieke Vandamme added a comment -

        I'm also using Firefox, but testing the same in Internet Explorer, gives the same output.
        I think it has nothing to do with the browser.
        The html that wicket generates isn't correct. Before printing < sty le type="text/css" media="print" >, there is no closing tag (< / sty le>).
        Also the same worked perfectly with wicket 1.5.7, also in Firefox.

        Show
        Marieke Vandamme added a comment - I'm also using Firefox, but testing the same in Internet Explorer, gives the same output. I think it has nothing to do with the browser. The html that wicket generates isn't correct. Before printing < sty le type="text/css" media="print" >, there is no closing tag (< / sty le>). Also the same worked perfectly with wicket 1.5.7, also in Firefox.
        Hide
        Sven Meier added a comment -

        Which browser are you using?

        When I check the response in Firefox' Net console, the generated markup looks fine. It seems that Firefox doesn't like @import inside the commented/CDATA style tags.

        BTW moving the markup from BasePage to Homepage (i.e. no markup inheritance) the outcome is the same.

        Show
        Sven Meier added a comment - Which browser are you using? When I check the response in Firefox' Net console, the generated markup looks fine. It seems that Firefox doesn't like @import inside the commented/CDATA style tags. BTW moving the markup from BasePage to Homepage (i.e. no markup inheritance) the outcome is the same.
        Marieke Vandamme made changes -
        Field Original Value New Value
        Attachment myproject.zip [ 12545540 ]
        Hide
        Marieke Vandamme added a comment -

        small quickstart that simulates the problem.

        Show
        Marieke Vandamme added a comment - small quickstart that simulates the problem.
        Marieke Vandamme created issue -

          People

          • Assignee:
            Emond Papegaaij
            Reporter:
            Marieke Vandamme
          • Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development