ICEfaces
  1. ICEfaces
  2. ICE-8131

Add annotations for browser conditional resource loading

    Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 3.3
    • Component/s: ACE-Components, Framework
    • Labels:
      None
    • Environment:
      IF 3.x
    • Assignee Priority:
      P1

      Description

      This issue encompasses the work done to implement annotations that allow definition of browser specific resources. Also detailed is the work to abstract browser detection to the core, as well as add an improved API over the original ResourceDependency annotations.

      The original scope of this issue was:
      Allow app developer specified or automatic inclusion in IE 7/8 of excanvas.js to chart component dependencies.

      This sizable dependency should be configurable so that it can be removed when possible, but our dependency annotation doesn't support runtime toggling.

        Activity

        Hide
        Ken Fyten added a comment -

        In order to do this we should enhance the ACE resource annotations to optionally support conditional resource loading for specific platforms, such as IE7/8. There is a similar mechanism in the ICEmobile code (deviceResource component).

        Show
        Ken Fyten added a comment - In order to do this we should enhance the ACE resource annotations to optionally support conditional resource loading for specific platforms, such as IE7/8. There is a similar mechanism in the ICEmobile code (deviceResource component).
        Hide
        Nils Lundquist added a comment -

        ace:chart now uses new ICEResourceDependency to conditionally load the excanvas library.

        ICEResourceDependency is functionally complete, though some of the API may be renamed before this JIRA is closed.

        Also with UserAgentInfo now being available in core, ICEMobile should be update to use it.

        Show
        Nils Lundquist added a comment - ace:chart now uses new ICEResourceDependency to conditionally load the excanvas library. ICEResourceDependency is functionally complete, though some of the API may be renamed before this JIRA is closed. Also with UserAgentInfo now being available in core, ICEMobile should be update to use it.
        Hide
        Nils Lundquist added a comment -

        Revision #33244
        Committed by nils.lundquist
        A minute ago
        ICE-8131 - ace:chart now conditionally loads excanvas IE7/8 compat library. Added new ICEResourceDependency annotation to replace use of ResourceDependency and enable definition of browser specific resources. Replacement was necessary due to the restrictions of annotations.
        Ported UserAgentInfo.java from ICEMobile to Core.

        Show
        Nils Lundquist added a comment - Revision #33244 Committed by nils.lundquist A minute ago ICE-8131 - ace:chart now conditionally loads excanvas IE7/8 compat library. Added new ICEResourceDependency annotation to replace use of ResourceDependency and enable definition of browser specific resources. Replacement was necessary due to the restrictions of annotations. Ported UserAgentInfo.java from ICEMobile to Core.
        Hide
        Nils Lundquist added a comment -

        I'll post a few annotations here for demonstration:

        @ICEResourceDependencies({
                @ICEResourceDependency(library = "icefaces.ace", name = "util/ace-jquery.js"),
                @ICEResourceDependency(library = "icefaces.ace", name = "chart/excanvas.js", browser = BrowserType.IE8_OR_LESS),
                @ICEResourceDependency(library = "icefaces.ace", name = "chart/ace-chart.js"),
                @ICEResourceDependency(library = "icefaces.ace", name = "util/combined.css")
        })
        

        is equiv to:

        @ICEResourceDependencies({
            @ICEResourceDependency(library = "icefaces.ace", name = "util/ace-jquery.js"),
            @ICEResourceDependency(browserOverride = {
                @ICEBrowserDependency(browser = BrowserType.IE8_OR_LESS, library = "icefaces.ace", name = "chart/excanvas.js")
            }),
            @ICEResourceDependency(library = "icefaces.ace", name = "chart/ace-chart.js"),
            @ICEResourceDependency(library = "icefaces.ace", name = "util/combined.css")
        })
        
        @ICEResourceDependencies({
            @ICEResourceDependency(library = "icefaces.ace", name = "util/ace-jquery.js"),
            @ICEResourceDependency(
                library = "icefaces.ace", name = "chart/default-excanvas.js", // Default resource
                browserOverride = { // Alternate resources
                    @ICEBrowserDependency(browser = BrowserType.IE8_OR_LESS, library = "icefaces.ace", name = "chart/old-ie-excanvas.js"), // Alt resource
                    @ICEBrowserDependency(browser = BrowserType.IE9_OR_GREATER) // Opt out of resource
                }
            }),
            @ICEResourceDependency(library = "icefaces.ace", name = "chart/ace-chart.js"),
            @ICEResourceDependency(library = "icefaces.ace", name = "util/combined.css")
        })
        
        Show
        Nils Lundquist added a comment - I'll post a few annotations here for demonstration: @ICEResourceDependencies({ @ICEResourceDependency(library = "icefaces.ace" , name = "util/ace-jquery.js" ), @ICEResourceDependency(library = "icefaces.ace" , name = "chart/excanvas.js" , browser = BrowserType.IE8_OR_LESS), @ICEResourceDependency(library = "icefaces.ace" , name = "chart/ace-chart.js" ), @ICEResourceDependency(library = "icefaces.ace" , name = "util/combined.css" ) }) is equiv to: @ICEResourceDependencies({ @ICEResourceDependency(library = "icefaces.ace" , name = "util/ace-jquery.js" ), @ICEResourceDependency(browserOverride = { @ICEBrowserDependency(browser = BrowserType.IE8_OR_LESS, library = "icefaces.ace" , name = "chart/excanvas.js" ) }), @ICEResourceDependency(library = "icefaces.ace" , name = "chart/ace-chart.js" ), @ICEResourceDependency(library = "icefaces.ace" , name = "util/combined.css" ) }) @ICEResourceDependencies({ @ICEResourceDependency(library = "icefaces.ace" , name = "util/ace-jquery.js" ), @ICEResourceDependency( library = "icefaces.ace" , name = "chart/ default -excanvas.js" , // Default resource browserOverride = { // Alternate resources @ICEBrowserDependency(browser = BrowserType.IE8_OR_LESS, library = "icefaces.ace" , name = "chart/old-ie-excanvas.js" ), // Alt resource @ICEBrowserDependency(browser = BrowserType.IE9_OR_GREATER) // Opt out of resource } }), @ICEResourceDependency(library = "icefaces.ace" , name = "chart/ace-chart.js" ), @ICEResourceDependency(library = "icefaces.ace" , name = "util/combined.css" ) })
        Hide
        Nils Lundquist added a comment -

        I like most of the naming conventions of this code, with the exception of 'browserOverride', any suggestions?

        Show
        Nils Lundquist added a comment - I like most of the naming conventions of this code, with the exception of 'browserOverride', any suggestions?
        Hide
        Ken Fyten added a comment -

        browserOverride is an apt name, I don't have an issue with it since it does override the default resource.

        Show
        Ken Fyten added a comment - browserOverride is an apt name, I don't have an issue with it since it does override the default resource.
        Hide
        Nils Lundquist added a comment -

        One remaining question is if the user agent results should be cached for the session. It currently isn't due to session size concerns, but it would eliminate repeated String comparisons in RestoreResourceDependencies.

        Show
        Nils Lundquist added a comment - One remaining question is if the user agent results should be cached for the session. It currently isn't due to session size concerns, but it would eliminate repeated String comparisons in RestoreResourceDependencies.
        Hide
        Ken Fyten added a comment -

        Seems like a legit use of Session scope to me. Won't take much space if it's a Boolean.

        Show
        Ken Fyten added a comment - Seems like a legit use of Session scope to me. Won't take much space if it's a Boolean.
        Hide
        Nils Lundquist added a comment -

        create new tests for UserAgentInfo

        Show
        Nils Lundquist added a comment - create new tests for UserAgentInfo
        Hide
        Mark Collette added a comment -

        If browserOverride is for when we want to use one resource, or alternatively another one, then what is the means for simply adding resources that are browser specific. For example, let's say I have a base.css that I always want served, and when I have IE7 I additionally want another css file served afterwards.

        It would be nice if this took into consideration ICE-8784.

        Show
        Mark Collette added a comment - If browserOverride is for when we want to use one resource, or alternatively another one, then what is the means for simply adding resources that are browser specific. For example, let's say I have a base.css that I always want served, and when I have IE7 I additionally want another css file served afterwards. It would be nice if this took into consideration ICE-8784.
        Hide
        Ted Goddard added a comment -

        It should be possible to additionally factor the annotation to be less verbose; in most cases, the library for all resources is common.

        @ICEResourceConfig(library = "icefaces.ace")
        @ICEResourceDependencies({
        @ICEResourceDependency(
        names =

        {"util/ace-jquery.js", "chart/ace-chart.js", "util/combined.css"}

        ),
        @ICEResourceDependency(name = "chart/excanvas.js",
        browser = BrowserType.IE8_OR_LESS),
        })

        Type safety can be improved as follows:

        @ICEResourceConfig(library = AceResources.LIBRARY)
        @ICEResourceDependencies({
        @ICEResourceDependency(
        names =

        {AceResources.JQUERY, "chart/ace-chart.js", AceResources.COMBINED}

        ),
        @ICEResourceDependency(name = "chart/excanvas.js",
        browser = BrowserType.IE8_OR_LESS),
        })

        Show
        Ted Goddard added a comment - It should be possible to additionally factor the annotation to be less verbose; in most cases, the library for all resources is common. @ICEResourceConfig(library = "icefaces.ace") @ICEResourceDependencies({ @ICEResourceDependency( names = {"util/ace-jquery.js", "chart/ace-chart.js", "util/combined.css"} ), @ICEResourceDependency(name = "chart/excanvas.js", browser = BrowserType.IE8_OR_LESS), }) Type safety can be improved as follows: @ICEResourceConfig(library = AceResources.LIBRARY) @ICEResourceDependencies({ @ICEResourceDependency( names = {AceResources.JQUERY, "chart/ace-chart.js", AceResources.COMBINED} ), @ICEResourceDependency(name = "chart/excanvas.js", browser = BrowserType.IE8_OR_LESS), })
        Hide
        Nils Lundquist added a comment -

        Revision #33286
        Committed by nils.lundquist
        A minute ago
        ICE-8131 - Removed MyFaces compatibility hack that was causing issues in Mojarra 2.1.7 and earlier. No longer needed since we don't use standard ResourceDependency and ResourceDependencies annotations.

        Show
        Nils Lundquist added a comment - Revision #33286 Committed by nils.lundquist A minute ago ICE-8131 - Removed MyFaces compatibility hack that was causing issues in Mojarra 2.1.7 and earlier. No longer needed since we don't use standard ResourceDependency and ResourceDependencies annotations.
        Hide
        Ken Fyten added a comment -
        • Implement core caching of user-agent details.
        • Add static strings for type safety.
        Show
        Ken Fyten added a comment - Implement core caching of user-agent details. Add static strings for type safety.
        Hide
        Nils Lundquist added a comment -

        Revision #33362
        Committed by nils.lundquist
        Yesterday 3:38 PM
        ICE-8131 - Addde ICEResouceLibrary and ACEResourceNames, used to simplfy ResourceDependency definitions.

        Revision #33369
        Committed by nils.lundquist
        3 minutes ago
        ICE-8131 - Added UserAgentContext util to add caching to UserAgentInfo browser / device detection.

        Show
        Nils Lundquist added a comment - Revision #33362 Committed by nils.lundquist Yesterday 3:38 PM ICE-8131 - Addde ICEResouceLibrary and ACEResourceNames, used to simplfy ResourceDependency definitions. Revision #33369 Committed by nils.lundquist 3 minutes ago ICE-8131 - Added UserAgentContext util to add caching to UserAgentInfo browser / device detection.
        Hide
        Nils Lundquist added a comment -

        should this JIRA remain open until a MOBI JIRA is opened to adopt these core additions?

        Show
        Nils Lundquist added a comment - should this JIRA remain open until a MOBI JIRA is opened to adopt these core additions?
        Hide
        Nils Lundquist added a comment -

        This issue is ready to be closed pending adoption of these changes by mobi.

        Show
        Nils Lundquist added a comment - This issue is ready to be closed pending adoption of these changes by mobi.
        Hide
        Nils Lundquist added a comment -

        Porting complete.

        Adoption of changes in mobi covered under: MOBI-654

        Show
        Nils Lundquist added a comment - Porting complete. Adoption of changes in mobi covered under: MOBI-654

          People

          • Assignee:
            Nils Lundquist
            Reporter:
            Nils Lundquist
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: