Tapestry 5
  1. Tapestry 5
  2. TAP5-184

Improve error reporting when a javascript asset is intended to be included on page which has no <html> element

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 5.0.15
    • Fix Version/s: 5.0.16
    • Component/s: None
    • Labels:
      None

      Description

      I know - silly of me to have a page with no <body></body> element. I spent an entire day trying to figure out why the javascript wasn't getting included.

      It was my error, but I can see other people making the same mistake...a little error-reporting would be nice.

        Activity

        Michael Lake created issue -
        Howard M. Lewis Ship made changes -
        Field Original Value New Value
        Component/s tapestry-core [ 12311285 ]
        Key TAPESTRY-2031 TAP5-184
        Project Tapestry [ 10573 ] Tapestry 5 [ 12310833 ]
        Affects Version/s 5.0.8 [ 12312898 ]
        Howard M. Lewis Ship made changes -
        Affects Version/s 5.0.15 [ 12313429 ]
        Howard M. Lewis Ship made changes -
        Assignee Howard M. Lewis Ship [ hlship ]
        Hide
        Howard M. Lewis Ship added a comment -

        Yes; I think this should be an error, not just skipped logic. It would be clearer as an error.

        Show
        Howard M. Lewis Ship added a comment - Yes; I think this should be an error, not just skipped logic. It would be clearer as an error.
        Hide
        Fernando Padilla added a comment -

        The thing is that we don't just publish to html, we also publish to html fragments (<div>), fbml (facebook, <fbml>), and even json (packaging kind of like partial render). So i want to make sure that if you do decide this is an error, not make it a fatal error, and make it so we can turn off the logging on it, since that might fail often.

        Also, we had a hard time trying to properly override the RenderSupport and MarkupRenderFilter structure to properly handle the other options of rendering. If we had a good way to change contributions it would be much easier (we could replace standard RenderSupport with ours). Or if the RenderSupport was somehow a service we could decorate it would be much easier, etc etc..

        Show
        Fernando Padilla added a comment - The thing is that we don't just publish to html, we also publish to html fragments (<div>), fbml (facebook, <fbml>), and even json (packaging kind of like partial render). So i want to make sure that if you do decide this is an error, not make it a fatal error, and make it so we can turn off the logging on it, since that might fail often. Also, we had a hard time trying to properly override the RenderSupport and MarkupRenderFilter structure to properly handle the other options of rendering. If we had a good way to change contributions it would be much easier (we could replace standard RenderSupport with ours). Or if the RenderSupport was somehow a service we could decorate it would be much easier, etc etc..
        Hide
        Howard M. Lewis Ship added a comment -

        The case described here concerns just full page renders, so HTML fragments (as part of an Ajax request) are handled quite differently.

        I'm looking at http://wiki.developers.facebook.com/index.php/FBML but can't quite figure out the structure of the document.

        That being said, I believe what I'll be doing is making it easier to override the DocumentLinker, which will be its own Environmental object, seperate from the RenderSupport. This will make it easier to keep Tapestry's logic as is, but change the rules for how the CSS links, JavaScript links and initialization JavaScript are linked (or "integrated") with the overall page. Eventually, some of these interfaces will migrate out to the public API space.

        Show
        Howard M. Lewis Ship added a comment - The case described here concerns just full page renders, so HTML fragments (as part of an Ajax request) are handled quite differently. I'm looking at http://wiki.developers.facebook.com/index.php/FBML but can't quite figure out the structure of the document. That being said, I believe what I'll be doing is making it easier to override the DocumentLinker, which will be its own Environmental object, seperate from the RenderSupport. This will make it easier to keep Tapestry's logic as is, but change the rules for how the CSS links, JavaScript links and initialization JavaScript are linked (or "integrated") with the overall page. Eventually, some of these interfaces will migrate out to the public API space.
        Hide
        Howard M. Lewis Ship added a comment -

        In terms of overrides; my intention was that you could place a "competing" filter into the pipeline just after the filter being replaced. It would, in effect, intercept all the calls to the built-in filter ... in other words, Tapestry's RenderSupport instance would be present, but would never be invoked, because your custom, Facebook-compatible RenderSupport instance would be the one visible to the rest of the application during the render.

        Show
        Howard M. Lewis Ship added a comment - In terms of overrides; my intention was that you could place a "competing" filter into the pipeline just after the filter being replaced. It would, in effect, intercept all the calls to the built-in filter ... in other words, Tapestry's RenderSupport instance would be present, but would never be invoked, because your custom, Facebook-compatible RenderSupport instance would be the one visible to the rest of the application during the render.
        Hide
        Fernando Padilla added a comment -

        the fbml document is really like an xhtml fragment that allows elements from the facebook namespace. So the only thing is that the RederSupport needs to know about <fbml> rather than <html><head><body>.

        Yes I kind of figured out that the DocumentLinker was the thing that needed to be overridden, but the way it was coded, it was privately owned and used by RenderSupport.. so I had to override RenderSupport, to be able to override DocumentLinker.

        Show
        Fernando Padilla added a comment - the fbml document is really like an xhtml fragment that allows elements from the facebook namespace. So the only thing is that the RederSupport needs to know about <fbml> rather than <html><head><body>. Yes I kind of figured out that the DocumentLinker was the thing that needed to be overridden, but the way it was coded, it was privately owned and used by RenderSupport.. so I had to override RenderSupport, to be able to override DocumentLinker.
        Hide
        Howard M. Lewis Ship added a comment -

        Right ... I want to offer the ability to override DocumentLinker seperately from RenderSupport (so that you don't need to change RenderSupport).

        Show
        Howard M. Lewis Ship added a comment - Right ... I want to offer the ability to override DocumentLinker seperately from RenderSupport (so that you don't need to change RenderSupport).
        Hide
        Fernando Padilla added a comment -

        I know that your intention was to allow people to place a filter after the real RenderSupport and allow easy overriding that way, but it did not pan out that way for various reasons:

        1) There was no way for me to turn off RenderSupport, in that i always did work, even if I took over much of the work for it.. ( it still called 'commit' and 'render' on the way out of the markupfilter..
        2) The other markupfilters that depend on RenderSupport know about that ( before:RenderSupport, after:RenderSupport ), but do not know about my secondary overriding filter.. so it's unreliable to me to override it.. basically i would need a third option ( directly-after:RenderSupport, override:RenderSupport ), to make sure that my Filter went right after RenderSupport, but before anyone that was depending on it..

        Because of these things, it still boils down to the configuration system is trying to hide too many things from people using it.. And I hold that not exposing a clean way to override or change configurations made by underlying libraries is a deficiency.. We can come up with work arounds and various other ideas, but I think we'll always come back to this.

        Show
        Fernando Padilla added a comment - I know that your intention was to allow people to place a filter after the real RenderSupport and allow easy overriding that way, but it did not pan out that way for various reasons: 1) There was no way for me to turn off RenderSupport, in that i always did work, even if I took over much of the work for it.. ( it still called 'commit' and 'render' on the way out of the markupfilter.. 2) The other markupfilters that depend on RenderSupport know about that ( before:RenderSupport, after:RenderSupport ), but do not know about my secondary overriding filter.. so it's unreliable to me to override it.. basically i would need a third option ( directly-after:RenderSupport, override:RenderSupport ), to make sure that my Filter went right after RenderSupport, but before anyone that was depending on it.. Because of these things, it still boils down to the configuration system is trying to hide too many things from people using it.. And I hold that not exposing a clean way to override or change configurations made by underlying libraries is a deficiency.. We can come up with work arounds and various other ideas, but I think we'll always come back to this.
        Hide
        Fernando Padilla added a comment -

        If you're going to be enhancing the DocumentLinker, please make it mutable as well

        This allows us to be much more fancy stuff. Like having a service that reviews the desired javascript and replaces the url with a compressed version. Or it can collapse various javascript assets into one ( like YUI does ), or.. or.. or.. All without having to make your own DocumentLinker, simply looking into it's list of assets and modifying it as required.

        Show
        Fernando Padilla added a comment - If you're going to be enhancing the DocumentLinker, please make it mutable as well This allows us to be much more fancy stuff. Like having a service that reviews the desired javascript and replaces the url with a compressed version. Or it can collapse various javascript assets into one ( like YUI does ), or.. or.. or.. All without having to make your own DocumentLinker, simply looking into it's list of assets and modifying it as required.
        Howard M. Lewis Ship made changes -
        Status Open [ 1 ] In Progress [ 3 ]
        Hide
        Howard M. Lewis Ship added a comment -

        I've split DocumentLinker from RenderSupport.

        In RenderSupportImpl:

        public void commit()
        {
        if (focusFieldId != null)

        { addScript("$('%s').activate();", focusFieldId); }

        if (init.length() > 0)

        { addScript("Tapestry.init(%s);", init); }

        }

        This means that if you provide an alternate version of RenderSupport, and the default one receives no method invocations, then the commit() will do nothing. So it is replaceable.

        In terms of making DL "mutable" ... we're sharing some ideas for 5.1 (or 5.2, depending). Anyway, the point is, you can replace the DocumentLinker implementation in the Environment before the RenderSupport filter grabs it, and your implementation can do whatever. In terms of Facebook support, you'll want an implementation that expects the root element to be <fbml> and put the necessary <link> and <script> tags there.

        Show
        Howard M. Lewis Ship added a comment - I've split DocumentLinker from RenderSupport. In RenderSupportImpl: public void commit() { if (focusFieldId != null) { addScript("$('%s').activate();", focusFieldId); } if (init.length() > 0) { addScript("Tapestry.init(%s);", init); } } This means that if you provide an alternate version of RenderSupport, and the default one receives no method invocations, then the commit() will do nothing. So it is replaceable. In terms of making DL "mutable" ... we're sharing some ideas for 5.1 (or 5.2, depending). Anyway, the point is, you can replace the DocumentLinker implementation in the Environment before the RenderSupport filter grabs it, and your implementation can do whatever. In terms of Facebook support, you'll want an implementation that expects the root element to be <fbml> and put the necessary <link> and <script> tags there.
        Hide
        Howard M. Lewis Ship added a comment -

        Tapestry will no require a root <html> element (and report it, if missing). It will create <head> and <body> elements within the root element, as needed.

        Show
        Howard M. Lewis Ship added a comment - Tapestry will no require a root <html> element (and report it, if missing). It will create <head> and <body> elements within the root element, as needed.
        Howard M. Lewis Ship made changes -
        Summary Improve error reporting when a javascript asset is intended to be included on page which has no <body> element Improve error reporting when a javascript asset is intended to be included on page which has no <html> element
        Howard M. Lewis Ship made changes -
        Status In Progress [ 3 ] Closed [ 6 ]
        Resolution Fixed [ 1 ]
        Fix Version/s 5.0.16 [ 12313427 ]

          People

          • Assignee:
            Howard M. Lewis Ship
            Reporter:
            Michael Lake
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development