Uploaded image for project: 'Tapestry'
  1. Tapestry
  2. TAPESTRY-2033

Optimized requests paths are broken for root paths within contexts

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 5.0.8
    • Fix Version/s: 5.0.8
    • Component/s: None
    • Labels:
      None

      Description

      TAPESTRY-1502 changed the way that URLs were generated. The comments on the issue indicate it works great for a non-root context, but my experience is indicating otherwise. It is possible I've done something wrong, but without changing a single line of code, all of my pagelinks, actionlinks, includescripts, includestylesheets, and so forth have broken.

      In this case, I have my app mapped to the "/manager" context. I access it locally via http://localhost:8080/manager/.

      The generated URLs are all of the form:

      <script src="manager/assets/scriptaculous/prototype.js" type="text/javascript"></script>

      As you can see, the context is being included in the path but done so relatively. The above would resolve to http://localhost:8080/manager/manager/assets/scriptaculous/prototype.js, which is incorrect. It should be http://localhost:8080/manager/assets/scriptaculous/prototype.js

        Activity

        Hide
        nirvdrum Kevin Menard added a comment -

        Looks like it's an off-by-one error. I'll prepare a patch.

        Show
        nirvdrum Kevin Menard added a comment - Looks like it's an off-by-one error. I'll prepare a patch.
        Hide
        nirvdrum Kevin Menard added a comment -

        This code removes a "-1" check. Presumably this code was in there in order to prevent an AIOOB exception, but the rest of the code makes that impossible anyway.

        The problem, using the example above, is that I would have the following:

        requestTerms[0] == ""
        requestTerms[1] == "manager"

        pathTerms[0] == ""
        pathTerms[1] == "manager"
        pathTerms[2] == "assets"
        .
        .
        .

        The first path through the loop would detect that "" was duplicated, but the second pass would not detect that "manager" was duplicated because the first conditional short-circuited the whole loop (i == 1; requestTerms.length = 2 - 1 == 1)

        Making this change fixed my app. Relative URLs seem to work throughout with "../" being added as appropriate.

        Show
        nirvdrum Kevin Menard added a comment - This code removes a "-1" check. Presumably this code was in there in order to prevent an AIOOB exception, but the rest of the code makes that impossible anyway. The problem, using the example above, is that I would have the following: requestTerms [0] == "" requestTerms [1] == "manager" pathTerms [0] == "" pathTerms [1] == "manager" pathTerms [2] == "assets" . . . The first path through the loop would detect that "" was duplicated, but the second pass would not detect that "manager" was duplicated because the first conditional short-circuited the whole loop (i == 1; requestTerms.length = 2 - 1 == 1) Making this change fixed my app. Relative URLs seem to work throughout with "../" being added as appropriate.
        Hide
        nirvdrum Kevin Menard added a comment -

        Bah. That induces a test failure. I missed it because I still have pending changes in my local checkout.

        I'll get a new patch that either fixes the failing test or fixes the approach taken.

        Show
        nirvdrum Kevin Menard added a comment - Bah. That induces a test failure. I missed it because I still have pending changes in my local checkout. I'll get a new patch that either fixes the failing test or fixes the approach taken.
        Hide
        hlship Howard M. Lewis Ship added a comment -

        I discoverred a flaw in the unit tests along the way. I have the fix just about together.

        Show
        hlship Howard M. Lewis Ship added a comment - I discoverred a flaw in the unit tests along the way. I have the fix just about together.

          People

          • Assignee:
            hlship Howard M. Lewis Ship
            Reporter:
            nirvdrum Kevin Menard
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development