Wicket
  1. Wicket
  2. WICKET-1507

MarkupCache style/variation/locale support broken

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 1.3.2
    • Fix Version/s: 1.3.4, 1.4-M2
    • Component/s: wicket
    • Labels:
      None

      Description

      We're running into a problem with the change made for this issue: WICKET-1370

      Basically, if you have a situation like this where the Login page extends DefaultPage and uses <wicket:extend>:

      Login.html
      DefaultPage.html
      DefaultPage_style1.html
      DefaultPage_style2.html

      The markup for DefaultPage will always be based on the first style the site is hit with. This is because the location string ("Login.html") hasn't changed with the style change.

      1. WICKET-1507-testcase.patch
        3 kB
        Jeremy Thomerson
      2. WICKET-1507-FIX-and-TESTCASE.patch
        6 kB
        Jeremy Thomerson

        Issue Links

          Activity

          Hide
          Johan Compagner added a comment -

          i applied the patch, i changed it a little bit more so that the location string that a MergedMarkup will get it from the base Markup
          That base markup can i guess be a MergedMarkup again (extend/child/extend/child)

          Show
          Johan Compagner added a comment - i applied the patch, i changed it a little bit more so that the location string that a MergedMarkup will get it from the base Markup That base markup can i guess be a MergedMarkup again (extend/child/extend/child)
          Hide
          Jeremy Thomerson added a comment -

          This is a fix for this bug, including a testcase to reproduce it without the fix applied.
          Ran all tests, and they all still succeeded.

          Show
          Jeremy Thomerson added a comment - This is a fix for this bug, including a testcase to reproduce it without the fix applied. Ran all tests, and they all still succeeded.
          Hide
          Meetesh Karia added a comment -

          Thanks! If I get a chance, I'll dig into the code again and see if I have any suggestions on how we might be able to fix this.

          Show
          Meetesh Karia added a comment - Thanks! If I get a chance, I'll dig into the code again and see if I have any suggestions on how we might be able to fix this.
          Hide
          Jeremy Thomerson added a comment -

          This is the modified test case that does fail.

          Show
          Jeremy Thomerson added a comment - This is the modified test case that does fail.
          Hide
          Jeremy Thomerson added a comment -

          Thanks for the clarification - I can now reproduce by modifying the test I had. I will try to see if I can find a fix, or this will at least (hopefully) help one of the core committers by giving them a reproducible test to start with and fix. I'll update the patch above with the new one.

          Show
          Jeremy Thomerson added a comment - Thanks for the clarification - I can now reproduce by modifying the test I had. I will try to see if I can find a fix, or this will at least (hopefully) help one of the core committers by giving them a reproducible test to start with and fix. I'll update the patch above with the new one.
          Hide
          Meetesh Karia added a comment -

          I haven't had a chance to test this out yet but it appears as though the test case is testing a slightly different scenario which I would expect to work from looking at the code. In the example above, the page we're trying to load doesn't have different markup for style1 or style2 but the base does. It will take me some time to get set up to verify this myself, but if someone could change the testcase to only have src/test/java/org/apache/wicket/markup/MarkupInheritanceExtension_1.html without the separate style1 and style2 files, then I think that will illustrate the problem.

          Thanks

          Show
          Meetesh Karia added a comment - I haven't had a chance to test this out yet but it appears as though the test case is testing a slightly different scenario which I would expect to work from looking at the code. In the example above, the page we're trying to load doesn't have different markup for style1 or style2 but the base does. It will take me some time to get set up to verify this myself, but if someone could change the testcase to only have src/test/java/org/apache/wicket/markup/MarkupInheritanceExtension_1.html without the separate style1 and style2 files, then I think that will illustrate the problem. Thanks
          Hide
          Johan Compagner added a comment -

          So is this fixed or not?
          Can somebody who does see this in their webapp try it out with trunk/snapshot (1.3 or 1.4)?

          Show
          Johan Compagner added a comment - So is this fixed or not? Can somebody who does see this in their webapp try it out with trunk/snapshot (1.3 or 1.4)?
          Hide
          Jeremy Thomerson added a comment -

          I tried writing a test case that would reproduce this so that I could work on a fix. I could not reproduce with this test case, which I think covers the described situation. I am testing on TRUNK, which is currently 1.4-SNAPSHOT, and the issue was reported with 1.3.2, so it might have been fixed between now and then.

          Attached patch is the test case I wrote.

          Show
          Jeremy Thomerson added a comment - I tried writing a test case that would reproduce this so that I could work on a fix. I could not reproduce with this test case, which I think covers the described situation. I am testing on TRUNK, which is currently 1.4-SNAPSHOT, and the issue was reported with 1.3.2, so it might have been fixed between now and then. Attached patch is the test case I wrote.

            People

            • Assignee:
              Johan Compagner
              Reporter:
              Meetesh Karia
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development