Uploaded image for project: 'OFBiz'
  1. OFBiz
  2. OFBIZ-7758

jquery.jgrowl.min.css is linked twice in html page

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Trivial
    • Resolution: Fixed
    • Affects Version/s: Trunk
    • Fix Version/s: 16.11.01
    • Component/s: ALL APPLICATIONS
    • Labels:
      None
    • Sprint:
      Community Day 3 - 2016

      Description

      I am running on trunk rev 1751489 on tomahawk theme.

      In the browser, right-click and view the page source. There is duplicate linking of jquery.jgrowl.min.css i.e.

      <link rel="stylesheet" href="/images/jquery/plugins/jquery-jgrowl/1.4.1/jquery.jgrowl.min.css" type="text/css"/>
      <link rel="stylesheet" href="/images/jquery/plugins/jquery-jgrowl/1.4.1/jquery.jgrowl.min.css" type="text/css"/>
      
      1. OFBIZ-7758.patch
        14 kB
        Rohit Koushal
      2. ALT-OFBIZ-7758.patch
        13 kB
        Rohit Koushal

        Issue Links

          Activity

          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          I'd take care of it, but I rather want to put require.js in place to handle all these sorts of things...

          Show
          jacques.le.roux Jacques Le Roux added a comment - I'd take care of it, but I rather want to put require.js in place to handle all these sorts of things...
          Hide
          rohit.koushal Rohit Koushal added a comment -

          Hi Jacques Le Roux and James Yong,

          Found out the problematic area, In AbstractModelAction.java below code snippet causing the duplicacy in the JavaScripts and StyleSheets List.

                      // this is a hack for backward compatibility with the JPublish page object
                      Map<String, Object> page = UtilGenerics.checkMap(context.get("page"));
                      if (page != null) {
                          this.field.put(page, newValue);
                      }
          

          The duplicacy can be because of context, globalContext and page contain the reference of single object(i.e layoutSettings).

          I Don't know the consequences of removing the above code. So for now providing the alternate solution in the attached patch.

          Please let me know if we are good to remove the above code snippet.

          Show
          rohit.koushal Rohit Koushal added a comment - Hi Jacques Le Roux and James Yong , Found out the problematic area, In AbstractModelAction.java below code snippet causing the duplicacy in the JavaScripts and StyleSheets List. // this is a hack for backward compatibility with the JPublish page object Map< String , Object > page = UtilGenerics.checkMap(context.get( "page" )); if (page != null ) { this .field.put(page, newValue); } The duplicacy can be because of context, globalContext and page contain the reference of single object(i.e layoutSettings). I Don't know the consequences of removing the above code. So for now providing the alternate solution in the attached patch. Please let me know if we are good to remove the above code snippet.
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          Thanks Rohit, this will certainly help. I did not get a chance yet to seriously look at what using jrequire.js in OFBiz would mean...

          Show
          jacques.le.roux Jacques Le Roux added a comment - Thanks Rohit, this will certainly help. I did not get a chance yet to seriously look at what using jrequire.js in OFBiz would mean...
          Hide
          amardeepsj Amardeep Singh Jhajj added a comment -

          One more thing I want to add here that, require.js works well for js module loading. Please refer:

          http://requirejs.org/docs/faq-advanced.html#css

          and here we are talking about css file. Thanks.

          Show
          amardeepsj Amardeep Singh Jhajj added a comment - One more thing I want to add here that, require.js works well for js module loading. Please refer: http://requirejs.org/docs/faq-advanced.html#css and here we are talking about css file. Thanks.
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          Thanks Rohit, JPublish is already in the Attic https://cwiki.apache.org/confluence/display/OFBADMIN/OFBiz+Attic#OFBizAttic-JPublish
          So yes please go with your solution since it was only a hack for JPublish. I saw it's also in ModelMenuAction.java

          And thanks Amardeep for the link about jrequire.js and .css files. Good to know, I still did not get a chance to seriously look at jrequire.js :/

          Show
          jacques.le.roux Jacques Le Roux added a comment - Thanks Rohit, JPublish is already in the Attic https://cwiki.apache.org/confluence/display/OFBADMIN/OFBiz+Attic#OFBizAttic-JPublish So yes please go with your solution since it was only a hack for JPublish. I saw it's also in ModelMenuAction.java And thanks Amardeep for the link about jrequire.js and .css files. Good to know, I still did not get a chance to seriously look at jrequire.js :/
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          Hi Rohit, I saw you assigned yourself, something changed?

          Show
          jacques.le.roux Jacques Le Roux added a comment - Hi Rohit, I saw you assigned yourself, something changed?
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          Bump

          Show
          jacques.le.roux Jacques Le Roux added a comment - Bump
          Hide
          rohit.koushal Rohit Koushal added a comment - - edited

          Hi Jacques Le Roux

          Sorry for the delay. Here is the patch for the issue. Please review and let me know you find any problem in the fix.

          Show
          rohit.koushal Rohit Koushal added a comment - - edited Hi Jacques Le Roux Sorry for the delay. Here is the patch for the issue. Please review and let me know you find any problem in the fix.
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          Thanks Rohit,

          It was worth to wait. Your slightly modified patch is in trunk at revision: 1761235. I simply removed the parentheses around (title!) in ftl files.

          Since it's a trivial issue I'll not try to backport, with all these paths changes it would be a real nightmare!

          Show
          jacques.le.roux Jacques Le Roux added a comment - Thanks Rohit, It was worth to wait. Your slightly modified patch is in trunk at revision: 1761235. I simply removed the parentheses around (title!) in ftl files. Since it's a trivial issue I'll not try to backport, with all these paths changes it would be a real nightmare!
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          Thanks Jame for report!

          Show
          jacques.le.roux Jacques Le Roux added a comment - Thanks Jame for report!
          Hide
          rohit.koushal Rohit Koushal added a comment -
          Show
          rohit.koushal Rohit Koushal added a comment - Thanks Jacques Le Roux .

            People

            • Assignee:
              jacques.le.roux Jacques Le Roux
              Reporter:
              jamesyong James Yong
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development

                  Agile