Opened 8 years ago

Closed 7 years ago

#14789 closed defect (fixed)

Paths to dijit images broken in optimised css located outside of dojo tree

Reported by: Nick Fenwick Owned by: Rawld Gill
Priority: blocker Milestone: 1.8
Component: BuildSystem Version: 1.7.1
Keywords: Cc:
Blocked By: Blocking:

Description

I'm not sure if this is a bug, or something I should be working around with build profile options, e.g. 'files' or 'paths' or similar. I have a build tree like this (simplified, based on github/csnover/dojo-boilerplate):

/src/dojo /src/dijit /src/app/app.profile.js /custom/custom.css /custom/index.html

custom/custom.css contains, among other things, references to the dijit theme to use, e.g.

@import url("../src/dijit/themes/claro/claro.css")

The custom directory is included in the build by means of a 'packages' element in app.profile.js:

   packages: [ {
        name: 'custom',
        location: '../custom'
    } ],

In my full app, there is also two layers built based on dijits in /custom, as well as the custom.css file produced.

I find that the optimized custom.css file put in dist/custom/custom.css does have claro.css inline into it, but the .png files referenced are wrong. For example my browser, loading http://localhost/dojo-boilerplate/dist/custom/index.html, shows 404 errors for:

http://localhost/dojo-boilerplate/dist/src/dijit/themes/claro/form/images/button.png
http://localhost/dojo-boilerplate/dist/src/dijit/themes/claro/form/images/formHighlight.png
http://localhost/dojo-boilerplate/dist/src/dijit/themes/claro/form/images/commonFormArrows.png

These can be seen in the optimised custom.css file as:

background-image: url(../src/dijit/themes/claro/form/images/button.png);
background-image: url(../src/dijit/themes/claro/form/images/formHighlight.png);

and so on. This makes the dijits render without graphics, for example a dijit.form.Select has no 'down arrow'.

Is this a builder bug, or something that can be worked around, or a combination of the two?

Attachments (3)

14789_dojo-boilerplate_skeleton.tgz (3.2 KB) - added by Nick Fenwick 8 years ago.
Files to be overlaid into csnover/dojo-boilerplate to reproduce the problem.
cssPathFix.patch (1.1 KB) - added by Mike Wilcox 7 years ago.
Creates a property for CSS images paths
custom.zip (2.6 KB) - added by Rawld Gill 7 years ago.
demonstrates [29293] effectiveness

Download all attachments as: .zip

Change History (21)

Changed 8 years ago by Nick Fenwick

Files to be overlaid into csnover/dojo-boilerplate to reproduce the problem.

comment:1 Changed 8 years ago by Nick Fenwick

For the record, the URLs I quoted above work if the 'src/' is taken out.. that is, the broken URL:

http://localhost/dojo-boilerplate/dist/src/dijit/themes/claro/form/images/commonFormArrows.png

should be:

http://localhost/dojo-boilerplate/dist/dijit/themes/claro/form/images/commonFormArrows.png

So, the optimised css should not be:

background-image: url(../src/dijit/themes/claro/form/images/formHighlight.png);

But instead be:

background-image: url(../dijit/themes/claro/form/images/formHighlight.png);

comment:2 Changed 8 years ago by bill

Component: GeneralBuildSystem
Owner: set to Rawld Gill

comment:3 Changed 8 years ago by Rawld Gill

Status: newassigned

Changed 7 years ago by Mike Wilcox

Attachment: cssPathFix.patch added

Creates a property for CSS images paths

comment:4 Changed 7 years ago by Mike Wilcox

I've run across this problem too and would suggest the priority should be raised.

I have a patch that will help. It adds a cssImagePath property that you can include in your profile to instruct the builder to change the path to the images. This fixed my build. I'm not sure if this is a hack or a fix though. I concede that it may be very difficult for the builder to figure this out without help.

comment:5 Changed 7 years ago by Mike Wilcox

Oh, it may have been... "off topic", but that patch also includes a log for missing CSS files which is a huge help when developing a build. That should probably be an official warning in bc.log (which I can do).

comment:6 Changed 7 years ago by Nick Fenwick

Thanks Mike, I only just noticed this patch and have tried it out. I'm having trouble making this achieve anything useful. I think it's because some resources are from nested includes, which breaks with your patch.

e.g. unpatched, custom.css contains:

background-image: url(../src/dijit/themes/claro/form/images/button.png);

Putting cssImagePath: '../../' in the profile, with your patch in place, makes the custom.css file contain:

background-image: url(../../src/dijit/form/images/button.png)

i.e. it still has the 'src' component that caused the trouble in the first place.

The scenario is that a custom/custom.css file contains the import statement:

@import url("../src/dijit/themes/claro/claro.css");

claro.css imports form/Button.css, and it's doing the replacement trick for this included css, essentially dropping two directory levels (inserting ../../, and then squishing them away in a further optimisation that's done).

e.g. dijit/themes/claro/form/Button.css contains:

background-image: url("../form/images/button.png");

So ../form/images/button.png is having the leading ../ removed, then ../../ prepended, to give ../../form/images/button.png. This is then used in the flattened content for claro.css, so it has "background-image: url(../../src/dijit/themes/claro/../../form/images/button.png)" and this is then collapsed, losing the vital 'themes/claro/ part of the path.

I'm not sure a single build option such as cssImagePath is going to be a strong solution.

Did you work through the example files I attached? I just spent some time piecing the project back together to test this which was rather annoying, so I'll clarify the steps:

This will produce a 'dist' folder including dist/custom/custom.css where the problem exhibits itself.

I'm going to stick to my ant build script hack using <replace> for now.

comment:7 Changed 7 years ago by Mike Wilcox

Priority: undecidedhigh

Neek, due to limitations in the older build system, I had gotten into the habit of setting my directory structure a certain way. And now in 1.7, a certain structure is encouraged to work with packages. Dojo and Dijit should be on the same level as your app code - and they should be all thought of as discrete packages (or package-like).

At least, I'm guessing this may be a problem since you have a /src root.

Anyway, sorry - I kind of knew my patch was limited. I'm not the one who can fix this. Hopefully it will soon. I'll raise the priority.

comment:8 Changed 7 years ago by Nick Fenwick

Mike, thanks for your time on this. My experience is that concerns relating to CSS and building modules outside the core SDK path simply don't get much attention in the dojo project. I've been working around these issues for 4 years. I've now bitten the bullet, and reorganised my project so my custom modules are all located under the dojo SDK directory. This seems to sidestep all the problems I was having with paths to resources in optimised CSS files, because the paths are all the same for both unbuilt and built environments. This introduces all sorts of source control headaches for my git repo, managing core dojo and my custom modules in the same folder, but that's life I guess.

At least now I don't have to do the horrendous URL rewriting I had to maintain in my httpd config to make my dev environment operate correctly, to work around these shortcomings in the build system :) It was getting to be too much, I have several environments and the path mangling was getting pretty complex.

comment:9 Changed 7 years ago by Rawld Gill

Milestone: tbd1.8

comment:10 Changed 7 years ago by Rawld Gill

Resolution: fixed
Status: assignedclosed

In [29293]:

fixed optimizeCss transform to properly adjust relative urls no matter how the destination trees are arranged; fixes #14789; !strict

Changed 7 years ago by Rawld Gill

Attachment: custom.zip added

demonstrates [29293] effectiveness

comment:11 Changed 7 years ago by Rawld Gill

The optimizeCss transform was originally taken almost verbatim from 1.6. Since 1.6 was not designed to process files outside a very limited standard organization, the transform was inadequate for the 1.8 build system.

[29293] is essentially a rewrite of the transform. In also includes some additional error checking, which caught a few inconsistencies in a few dtk CSS files.

The attachment above can be unzipped and located as explained in the file "test.html" inside the zip. The demonstration includes a tree outside the dtk tree that includes a CSS that imports from within the dtk tree. The demonstration further includes a profile to build the demonstration. You can exercise the both the unbuilt and built versions (custom/test.html, and customRelease/custome/test.html, respectively) to see the desired effect (observe the CSS files in the network panel in the built version compared to the unbuilt version).

comment:12 Changed 7 years ago by bill

Priority: highblocker
Resolution: fixed
Status: closedreopened

In http://archive.dojotoolkit.org/nightly/dojotoolkit/dijit/themes/themeTester.html, dijit.css isn't getting loaded (which leads to all sorts of problem such as each menu item appearing on a separate line).

I notice the expansion of claro.css is missing quotes here:

@import url(../dijit.css)

I think that's the cause of the problem, and I assume it's from [29293].

Last edited 7 years ago by bill (previous) (diff)

comment:13 Changed 7 years ago by Rawld Gill

Status: reopenedassigned

The branch of code for cssImportIgnore for CSS that's resolvable as a relative resource does not write the required semicolon. This branch also does not quote the URL; however this should not cause a problem.

comment:14 Changed 7 years ago by Rawld Gill

Resolution: fixed
Status: assignedclosed

In [29409]:

fixed missing semicolon; added optional quotes to URLs; fixes #14789; !strict

comment:15 Changed 7 years ago by Mathevet julien

The commit In [29409] broke css in shorthand mode. It put a semicolon just after url

ndState{
background: transparent url("img/puces.png") no-repeat -39px 0; 
width: 18px; 
height: 18px; 
vertical-align: middle;
}

Eg, becomes:

.ndState{background: transparent url("img/puces.png"); no-repeat -39px 0; width: 18px; height: 18px; vertical-align:
 middle;}
Last edited 7 years ago by Mathevet julien (previous) (diff)

comment:16 Changed 7 years ago by Mathevet julien

Look for eg nihilo.css after built. It occurs too

comment:17 Changed 7 years ago by dylan

Resolution: fixed
Status: closedreopened

Based on previous feedback, looks like this one needs another fix to address the use case of an image url mid-css rule.

comment:18 Changed 7 years ago by Rawld Gill

Resolution: fixed
Status: reopenedclosed

In [29446]:

repair improper url translation; fixes #14789; !strict

Note: See TracTickets for help on using tickets.