OFBiz
  1. OFBiz
  2. OFBIZ-111

Using hardwired /images instead of <@ofbizContentUrl> in .ftl files

    Details

      Description

      There are some template and script files which use hardwired links, e.g. /images. This means that ofbizContentUrl is not taken into account and may not work if some components (especially images) are mounted at different locations.

      For ftl files <@ofbizContentUrl> should be used. I have no good idea what to do with javascript and css files.

      This is a list of files created by grep (grepping "\"/images" ) from application directory, which probably should be fixed.

      content/webapp/content/images/composite.js
      ecommerce/webapp/ecommerce/shoppinglist/editShoppingList.ftl
      ecommerce/webapp/ecommerce/cart/showcart.ftl
      ecommerce/webapp/ecommerce/includes/header.ftl
      ecommerce/webapp/ecommerce/order/splitship.ftl
      manufacturing/webapp/manufacturing/routing/EditRoutingTaskAssoc.ftl
      manufacturing/webapp/manufacturing/jobshopmgt/CreateProductionRun.ftl
      manufacturing/webapp/manufacturing/bom/EditProductBom.ftl
      order/webapp/ordermgr/entry/cart/showcartitems.ftl
      order/webapp/ordermgr/entry/cart/showcart.ftl
      order/webapp/ordermgr/entry/catalog/productsummary.ftl
      order/webapp/ordermgr/entry/catalog/productdetail.ftl
      order/webapp/ordermgr/entry/catalog/configproductdetail.ftl
      order/webapp/ordermgr/order/appendorderitem.ftl
      party/webapp/partymgr/party/EditPartyRelationships.ftl
      product/webapp/facility/returns/receiveReturn.ftl
      product/webapp/facility/facility/FindFacilityLocation.ftl
      product/webapp/facility/facility/EditFacilityGroups.ftl
      product/webapp/facility/shipment/QuickShipOrder.ftl
      product/webapp/facility/shipment/FindShipment.ftl
      product/webapp/facility/group/EditFacilityGroupMembers.ftl
      product/webapp/facility/group/EditFacilityGroupRollup.ftl
      product/webapp/facility/inventory/receiveInventory.ftl
      product/webapp/facility/inventory/TransferInventoryItem.ftl
      product/webapp/facility/inventory/EditInventoryItem.ftl
      product/webapp/catalog/product/EditProductQuickAdmin.ftl
      product/webapp/catalog/product/ApplyFeaturesFromCategory.ftl
      product/webapp/catalog/product/EditProductAssoc.ftl
      product/webapp/catalog/product/EditProductFeatures.ftl
      product/webapp/catalog/product/EditProductFacilityLocations.ftl
      product/webapp/catalog/find/keywordsearch.ftl
      product/webapp/catalog/promo/EditProductPromoStores.ftl
      product/webapp/catalog/store/EditProductStorePromos.ftl
      product/webapp/catalog/store/EditProductStoreSurveys.ftl
      product/webapp/catalog/category/EditCategoryProducts.ftl
      product/webapp/catalog/category/EditCategoryParties.ftl
      product/webapp/catalog/category/EditCategoryRollup.ftl
      product/webapp/catalog/category/EditCategoryProdCatalogs.ftl
      product/webapp/catalog/category/EditCategoryFeatureCats.ftl

      1. additionalContentUrl.patch
        3 kB
        Chris Howe
      2. contentUrl.patch
        73 kB
        Chris Howe

        Activity

        Hide
        Jacques Le Roux added a comment -

        Thanks Chris,

        You patch is in OFBiz rev. 507435

        Show
        Jacques Le Roux added a comment - Thanks Chris, You patch is in OFBiz rev. 507435
        Hide
        Jacques Le Roux added a comment -
        Show
        Jacques Le Roux added a comment - Re-opened at Chris demand : https://issues.apache.org/jira/browse/OFBIZ-111#action_12469511
        Hide
        Chris Howe added a comment -

        Thanks for getting to this Jacques!

        Would it be possible for you to reopen this issue as the search and replace doesn't take care of the images set by variable. The additionalContent.patch took care of those except that the patch is set from an absolute path and also the time separation probably will make chunk errors. I'll try to resubmit a patch this weekend covering those if no one gets to it by then. Thanks!

        Show
        Chris Howe added a comment - Thanks for getting to this Jacques! Would it be possible for you to reopen this issue as the search and replace doesn't take care of the images set by variable. The additionalContent.patch took care of those except that the patch is set from an absolute path and also the time separation probably will make chunk errors. I'll try to resubmit a patch this weekend covering those if no one gets to it by then. Thanks!
        Hide
        Jacques Le Roux added a comment -

        Thanks Eriks and Chris,

        Committed in revision 502282.

        Show
        Jacques Le Roux added a comment - Thanks Eriks and Chris, Committed in revision 502282.
        Hide
        Jacques Le Roux added a comment -

        OK Chris,

        I will make it by hand.

        Show
        Jacques Le Roux added a comment - OK Chris, I will make it by hand.
        Hide
        Chris Howe added a comment -

        Sorry Jacques, I missed your 1-14 comment.

        If the patch fails for you, it will likely fail for me too now. If you want, all it was was a global search and replace as pointed out in https://issues.apache.org/jira/browse/OFBIZ-111#action_12459252 . Otherwise, I'll try to sneak in some time this weekend to redo it.

        Show
        Chris Howe added a comment - Sorry Jacques, I missed your 1-14 comment. If the patch fails for you, it will likely fail for me too now. If you want, all it was was a global search and replace as pointed out in https://issues.apache.org/jira/browse/OFBIZ-111#action_12459252 . Otherwise, I'll try to sneak in some time this weekend to redo it.
        Hide
        Jacques Le Roux added a comment -

        Hi Crhis

        Have you had a chance to look at this issue ?

        Show
        Jacques Le Roux added a comment - Hi Crhis Have you had a chance to look at this issue ?
        Hide
        Jacques Le Roux added a comment -

        Chris,

        I tried to apply the 1st patch but I got 7 "hunk failed" (old file). Please could you make another updated patch ? Else I will try to deal manually with this one... BTW I did not try additionalContentUrl.patch.

        Thanks

        Show
        Jacques Le Roux added a comment - Chris, I tried to apply the 1st patch but I got 7 "hunk failed" (old file). Please could you make another updated patch ? Else I will try to deal manually with this one... BTW I did not try additionalContentUrl.patch. Thanks
        Hide
        Chris Howe added a comment -

        Can someone take action/review the patches provided on this one? Do the patches need to be broken up to be smaller chunks? Any guidance would be appreciated.

        Show
        Chris Howe added a comment - Can someone take action/review the patches provided on this one? Do the patches need to be broken up to be smaller chunks? Any guidance would be appreciated.
        Hide
        Chris Howe added a comment -

        That's a good point. I never thought about that. This should work with the JS ones as well. Although then the js and css have to be hosted on the machine with the screen. Which is fine as this is still a step in the right direction.

        On a side note, this would also work quite well with the ecomclone stuff David is working on to include external controller information inside a controller.xml file.

        Show
        Chris Howe added a comment - That's a good point. I never thought about that. This should work with the JS ones as well. Although then the js and css have to be hosted on the machine with the screen. Which is fine as this is still a step in the right direction. On a side note, this would also work quite well with the ecomclone stuff David is working on to include external controller information inside a controller.xml file.
        Hide
        Joe Eckard added a comment -

        I haven't tested it, but I'm pretty sure that [your url]/css will be cached also. For what its worth, you can name /css anything you want... say for example, /screen.css. (<request-map uri="screen.css">)

        If you want some more control over the caching, you can add a script that runs with the ftl template to set the HTTP response headers that control the caching suggestions (Cache-control, Pragma, Expires, etc.).

        Show
        Joe Eckard added a comment - I haven't tested it, but I'm pretty sure that [your url] /css will be cached also. For what its worth, you can name /css anything you want... say for example, /screen.css. (<request-map uri="screen.css">) If you want some more control over the caching, you can add a script that runs with the ftl template to set the HTTP response headers that control the caching suggestions (Cache-control, Pragma, Expires, etc.).
        Hide
        Chris Howe added a comment -

        Thanks Joe,

        While that will work, one of the major benefits of css is that it caches in the client's browser and therefor only has to be downloaded the one time. This technique would call a request for a new css file each and every time a request is made that contains <link rel="stylesheet" href="<@ofbizUrl>/css</@ofbizUrl>" type="text/css"/>

        Show
        Chris Howe added a comment - Thanks Joe, While that will work, one of the major benefits of css is that it caches in the client's browser and therefor only has to be downloaded the one time. This technique would call a request for a new css file each and every time a request is made that contains <link rel="stylesheet" href="<@ofbizUrl>/css</@ofbizUrl>" type="text/css"/>
        Hide
        Joe Eckard added a comment -

        Regarding the CSS files... you can also switch those to a freemarker / screen widget setup to get the transforms. You would then reference the new request map URL instead of the old static "screen.css".

        ... screen def

        <screen name="css">
        <section>
        <widgets>
        <platform-specific>
        <html>
        <html-template location="component://<name>/webapp/<name>/css/screen.ftl"/>
        </html>
        </platform-specific>
        </widgets>
        </section>
        </screen>

        ... request map

        <request-map uri="css">
        <response name="success" type="view" value="css"/>
        </request-map>

        ... view-map

        <view-map name="css" type="screen" page="component://<name>/widget/CommonScreens.xml#css" content-type="text/css"/>

        .. link

        <link rel="stylesheet" href="<@ofbizUrl>/css</@ofbizUrl>" type="text/css" />

        Show
        Joe Eckard added a comment - Regarding the CSS files... you can also switch those to a freemarker / screen widget setup to get the transforms. You would then reference the new request map URL instead of the old static "screen.css". ... screen def <screen name="css"> <section> <widgets> <platform-specific> <html> <html-template location="component://<name>/webapp/<name>/css/screen.ftl"/> </html> </platform-specific> </widgets> </section> </screen> ... request map <request-map uri="css"> <response name="success" type="view" value="css"/> </request-map> ... view-map <view-map name="css" type="screen" page="component://<name>/widget/CommonScreens.xml#css" content-type="text/css"/> .. link <link rel="stylesheet" href="<@ofbizUrl>/css</@ofbizUrl>" type="text/css" />
        Hide
        Jacques Le Roux added a comment -

        There is definitively something to be done here, I vote for (did not review any patches for now)

        Show
        Jacques Le Roux added a comment - There is definitively something to be done here, I vote for (did not review any patches for now)
        Hide
        Eriks Dobelis added a comment -

        Concerning JavaScript, I had an idea that in .FTL file you could assign content root to a JavaScript variable, somthing like:
        <script language="JavaScript" type="text/javascript">
        var ofbizcontenturl = "<@ofbizContentUrl>/</@ofbizContentUrl>"
        </script>

        Then in files like calendar1.js, we could write:
        var obj_calwindow = window.open(ofbizcontenturl+'images/' + 'calendar.html?date...

        This would need updating all .FTLs using calendar1.js and other JavaScript files.

        Show
        Eriks Dobelis added a comment - Concerning JavaScript, I had an idea that in .FTL file you could assign content root to a JavaScript variable, somthing like: <script language="JavaScript" type="text/javascript"> var ofbizcontenturl = "<@ofbizContentUrl>/</@ofbizContentUrl>" </script> Then in files like calendar1.js, we could write: var obj_calwindow = window.open(ofbizcontenturl+'images/' + 'calendar.html?date... This would need updating all .FTLs using calendar1.js and other JavaScript files.
        Hide
        Chris Howe added a comment -

        The second patch takes care of the situations like the CommonScreens.xml example you gave, but just from the applications components. The XML should be giving the relative path, and the template that actually contains the <img> tag should handle the transform. That's not to say it's there may not be offending code in a screen widget <image> tag or a form widget, but both of those should have the ability to handle the transform.

        Since CSS is not a scripting language, the only way you can update that is through a startup service that uses string buffer to rewrite the css file on startup.

        I don't believe calendar1.js has access to the freemarker transform, so this would need to be created with a startup service as well.

        Show
        Chris Howe added a comment - The second patch takes care of the situations like the CommonScreens.xml example you gave, but just from the applications components. The XML should be giving the relative path, and the template that actually contains the <img> tag should handle the transform. That's not to say it's there may not be offending code in a screen widget <image> tag or a form widget, but both of those should have the ability to handle the transform. Since CSS is not a scripting language, the only way you can update that is through a startup service that uses string buffer to rewrite the css file on startup. I don't believe calendar1.js has access to the freemarker transform, so this would need to be created with a startup service as well.
        Hide
        Eriks Dobelis added a comment -

        The patch is exactly what I was thinking about with regards to FTLs. But then there are XML, CSS and JS files. E.g. those 2 I mentioned above. Probably there are more of this type. Two more:

        framework/images/webapp/images/tabstyles.css:

        .tabdownleft

        { background-image: url(/images/tabs/tab_down_left.gif); background-color: #B4B0AA; }

        .tabdownright

        { background-image: url(/images/tabs/tab_down_right.gif); background-color: #B4B0AA; }

        .tabupcenter

        { font-family: Arial, Helvetica, sans-serif; background-image: url(/images/tabs/tab_up_fill.gif); background-color: #D4D0C8; font-size: 15pt; text-align: center; vertical-align: middle; color: #000000; }

        .tabupleft

        { background-image: url(/images/tabs/tab_up_left.gif); background-color: #D4D0C8; border-width: 0px; padding: 0px; }

        This is from framework/images/webapp/images/calendar1.js:
        var obj_calwindow = window.open('/images/' +
        'calendar.html?datetime=' + this.dt_current.valueOf()+ '&id=' + this.id,
        'Calendar', 'width=150,height='(this.time_comp ? 220 : 235)
        ',status=no,resizable=yes,top='my',left='mx',dependent=yes,alwaysRaised=yes'
        );

        Show
        Eriks Dobelis added a comment - The patch is exactly what I was thinking about with regards to FTLs. But then there are XML, CSS and JS files. E.g. those 2 I mentioned above. Probably there are more of this type. Two more: framework/images/webapp/images/tabstyles.css: .tabdownleft { background-image: url(/images/tabs/tab_down_left.gif); background-color: #B4B0AA; } .tabdownright { background-image: url(/images/tabs/tab_down_right.gif); background-color: #B4B0AA; } .tabupcenter { font-family: Arial, Helvetica, sans-serif; background-image: url(/images/tabs/tab_up_fill.gif); background-color: #D4D0C8; font-size: 15pt; text-align: center; vertical-align: middle; color: #000000; } .tabupleft { background-image: url(/images/tabs/tab_up_left.gif); background-color: #D4D0C8; border-width: 0px; padding: 0px; } This is from framework/images/webapp/images/calendar1.js: var obj_calwindow = window.open('/images/' + 'calendar.html?datetime=' + this.dt_current.valueOf()+ '&id=' + this.id, 'Calendar', 'width=150,height=' (this.time_comp ? 220 : 235) ',status=no,resizable=yes,top=' my ',left=' mx ',dependent=yes,alwaysRaised=yes' );
        Hide
        Chris Howe added a comment -

        additionalContentUrl.patch

        wraps the image references with <@ofbizContentUrl> </@ofbizContentUrl> for several image references that come from a variable.

        Show
        Chris Howe added a comment - additionalContentUrl.patch wraps the image references with <@ofbizContentUrl> </@ofbizContentUrl> for several image references that come from a variable.
        Hide
        Chris Howe added a comment -

        contentUrl.patch

        replaces all:

        <img src="/images/cal.gif" with <img src="<@ofbizContentUrl>/images/cal.gif</@ofbizContentUrl>"

        and

        <img src="/images/fieldlookup.gif"" with <img src="<@ofbizContentUrl>/images/fieldlookup.gif"</@ofbizContentUrl>"

        this surprisingly takes care of all of the offending ftl instances.

        Show
        Chris Howe added a comment - contentUrl.patch replaces all: <img src="/images/cal.gif" with <img src="<@ofbizContentUrl>/images/cal.gif</@ofbizContentUrl>" and <img src="/images/fieldlookup.gif"" with <img src="<@ofbizContentUrl>/images/fieldlookup.gif"</@ofbizContentUrl>" this surprisingly takes care of all of the offending ftl instances.
        Hide
        Eriks Dobelis added a comment -

        The problem is that there are image links which use neither <@ofbizContentUrl> nor <@ofblizUrl>, e.g.:
        applications/order/webapp/ordermgr/entry/cart/showcart.ftl:119:
        <img src="/images/fieldlookup.gif" width="15" height="14" border="0" alt="Click here For Field Lookup"/>

        I consider instances like these should be replaced with:
        <img src="<@ofbizContentUrl>/images/fieldlookup.gif</@ofbizContentUrl>" width="15" height="14" border="0" alt="Click here For Field Lookup"/>

        another example:
        framework/common/widget/CommonScreens.xml:
        <set field="layoutSettings.styleSheets[+0]" value="/images/maincss.css" global="true"/>

        another one:
        framework/images/webapp/images/ecommain.css:50:
        img

        { behavior: url("/images/pngbehavior.htc"); }

        If I want to mount all ofbiz not at root, but at, e.g. /clientname (i.e. images will be mounted at /clientname/images), then I have to replace manually all hardwired "/images" links, which is not the right way to do it.

        Show
        Eriks Dobelis added a comment - The problem is that there are image links which use neither <@ofbizContentUrl> nor <@ofblizUrl>, e.g.: applications/order/webapp/ordermgr/entry/cart/showcart.ftl:119: <img src="/images/fieldlookup.gif" width="15" height="14" border="0" alt="Click here For Field Lookup"/> I consider instances like these should be replaced with: <img src="<@ofbizContentUrl>/images/fieldlookup.gif</@ofbizContentUrl>" width="15" height="14" border="0" alt="Click here For Field Lookup"/> another example: framework/common/widget/CommonScreens.xml: <set field="layoutSettings.styleSheets [+0] " value="/images/maincss.css" global="true"/> another one: framework/images/webapp/images/ecommain.css:50: img { behavior: url("/images/pngbehavior.htc"); } If I want to mount all ofbiz not at root, but at, e.g. /clientname (i.e. images will be mounted at /clientname/images), then I have to replace manually all hardwired "/images" links, which is not the right way to do it.
        Hide
        Chris Howe added a comment -

        All of the <@ofbizContentUrl> are setup relative the root. This is necessary so that images can be stored in places other than the images for a custom deployment. For instance there are occurances of both:

        <@ofbizContentUrl>/images and
        <@ofbizContentUrl>/content

        so, if you wish to use the images in ofbiz in an OOTB way with the images being served elsewhere, you must maintain the /images directory structure and set your ofbizContentUrl to the parent directory.

        This should either be

        1) closed as won't fix
        2) create another freemarker transform to distinguish between /images and /content
        3) change <@ofbizContentUrl>/content to <@ofbizUrl>/content and drop the /images from the template file.

        I would opt for 3 but wouldn't want to begin this refactor without a concensus

        Show
        Chris Howe added a comment - All of the <@ofbizContentUrl> are setup relative the root. This is necessary so that images can be stored in places other than the images for a custom deployment. For instance there are occurances of both: <@ofbizContentUrl>/images and <@ofbizContentUrl>/content so, if you wish to use the images in ofbiz in an OOTB way with the images being served elsewhere, you must maintain the /images directory structure and set your ofbizContentUrl to the parent directory. This should either be 1) closed as won't fix 2) create another freemarker transform to distinguish between /images and /content 3) change <@ofbizContentUrl>/content to <@ofbizUrl>/content and drop the /images from the template file. I would opt for 3 but wouldn't want to begin this refactor without a concensus

          People

          • Assignee:
            Jacques Le Roux
            Reporter:
            Eriks Dobelis
          • Votes:
            2 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development