h5bp / html5-boilerplate

A professional front-end template for building fast, robust, and adaptable web apps or sites.
https://html5boilerplate.com/
MIT License
56.27k stars 12.2k forks source link

Build script not maintaining relative CSS Path #686

Closed cmalven closed 12 years ago

cmalven commented 12 years ago

So, here I go again, with another Build Script gotcha.

My CSS and Javascript files are located in the default "css" and "js".

I have a page located at resume/index.html (which is referenced in project.properties), and the CSS file is referenced as such:

`

<!-- end CSS-->`

When the build script runs, it does most things as it should, but it doesn't maintain the ../ in the CSS path, it instead changes it to css/blahblahblah.css. First of all, why doesn't this work? It correctly maintains the relative path for JS files, so why not CSS? Secondly, what can I do about this? Seems like it would be a very common situation.

I can't use root-relative paths in this situation. I tried finding an answer to this in Issues or the Wiki and didn't come up with anything. Any ideas?

metachris commented 12 years ago

I know this issue as well, and usually ending up modifying build.xml. A nicer approach would be a project.properties setting!

If you want to update build.xml, take a look at lines 619 and 626

cmalven commented 12 years ago

I'm far from an expert with Ant, but it has always been difficult for me to understand why the build script can't just replace the CSS filename without touching the path. I mean, we're already specifying the exact filename in project.properties. I'm sure the issue gets very complex with multiple files in multiple paths, but it really is a bewildering issue, and very few people are going to be confident modifying build.xml.

paulirish commented 12 years ago

this seems related to what @drublic has in #685..

cmalven commented 12 years ago

Yes, it is somewhat related. But in this case we're not talking about custom, site-wide prefixes, we're just talking about plain-jane, been-around-since-the-dawn-of-html relative paths. Maybe I'm mistaken, but it doesn't seem like a "custom prefix" in project.properties would address this issue. What if one page uses ../css/style.css and another, deeper page uses ../../../css/style.css. This might not be a great practice, but it could definitely happen, and I don't see how a custom-prefix property would fully address this.

paulirish commented 12 years ago

yup +1

drublic commented 12 years ago

@cmalven My solution in #685 does not address issues like relative paths that vary on a page. But it would be possible. If you have a custom path like ../../css/… and you want to use the build script you also define the comments surrounding the CSS-files. With my changes to build.xml it is possible to define ../../ in this comment and it should be maintained. But it does not maintain paths if you have one file in ../../css/ and another in ../css on the same page for instance. Hopefully this helps you.

darktable commented 12 years ago

I commited a small change to my main branch that retains the prefix from the style.css file when swapping for the concatenated style.css.

https://github.com/darktable/html5-boilerplate/commit/19a26c06e89919e7d456c0db823d37b8614bd4bb

The current behavior in 2.0 came over with the @import patch, because I wanted the style.css concat behavior to match the script.js concat behavior. It looks for a comment block and swaps everything inside that block for the new tag. This isn't as necessary for the style.css, because it's only a single file being swapped.

paulirish commented 12 years ago

Nice. I really like 19a26c06e89919e7d456c0db823d37b8614bd4bb from darktable. Let's solve this with that.

cmalven commented 12 years ago

Thanks, @darktable that's great. I do have a question, though. If this is intended to be a permanent solution to this problem, will people know to remove the comments for this situation?

darktable commented 12 years ago

If this change is pulled in those comments will just be ignored. It will only look for a tag that references "style.css" and swap that for the concatenated file name.

paulirish commented 12 years ago

boom. it's in.

this dovetailed nicely with 64561534a6ba651a557bc204c73e64df48f3a479 which was for #687

thx all!

drublic commented 12 years ago

Srys, can we reopen this please?!

With solving #685 and this one we have the problem that the css-path gets maintained two times.

Example: In project.properties we add css.prefix = <?php base_url() ?>css. In the "development" index.html we have the line

    <link rel="stylesheet" href="<?php base_url(); ?>css/style.css">

The build-script maintains the <?php base_url(); ?>css infront of style.css but it also prepends the css.prefix-property.

It would be possible to just lease the css.prefix out of the game if we have something like a prefix for the css file already or we revert 2fef4ad3ee16149a781f7db5f037f829f23633c2. But maybe this is not a solution…

To discuss…

kevva commented 12 years ago

Yeah I suggested this earlier today. Since the build script seem to work fine with prefixes directly in your references we don't really need to add them through project.properties. I know there were issues before with prefixes when you had to add them manually to your build.xml, but they are fixed now.

paulirish commented 12 years ago

concurrency is hard. >.<

sry about this.

@drublic my bad. i think we can kill the config option yeah

@darktable, work for you?

darktable commented 12 years ago

@paulirish no objections. I'm sure someone will find a reason to need it right after we delete it, but I can't think of a reason.

metachris commented 12 years ago

I think having the build script take the prefix from the reference would be the easiest, most straightforward way and we should remove the config from project.properties.

paulirish commented 12 years ago

divya just committed 9fe23b3f82bc552c6f89c8e183593592f07f341d and 94b065fb97c84683a8d8c9cd0ca1bad04182387c

which remove the extra prefixing stuff.. and also the fixup that darktable did with maintaining the reference path... that now drops the sha.css filename in place.

from here that looks all well and good so i think we're straightened out! i'll wait to get some thumbs up from ya'll before i close this off though :)

drublic commented 12 years ago

Thanks @nimbupani for these commits. Works great for CSS-files.

Buuuut… Seems like it's not working with the JS-files. In this case we have more than one file that is replaced (plugins.js and script.js). We could keep the option in project.properties for this.

kevva commented 12 years ago

yup, js breaks. so yeah either keep js prefixes in project.properties or maybe use the solution which you provided in #685 where you put your prefix in the comment, for some consistency, but only for js then ofc. i don't know which is best really.

drublic commented 12 years ago

In #685 we figured that keeping the comment-prefix-thing I suggested is not the best thing to do. Guess it's best to do something similar to "keeping the path in front of the file" like with CSS-files.

kevva commented 12 years ago

yes, that would be optimal, but is it possible to do it with multiple files?

darktable commented 12 years ago

Whoops! I thought @paulirish was just going to remove the css.prefix stuff. I hadn't made the prefix-hunting changes to the javascript block.

This commit fixes the script.js stuff so that it similarly pulls the path prefix off of the src="??/script.js" value: https://github.com/darktable/html5-boilerplate/blob/0ef8baba870ac0c8dfb0fd088ae549476669a261/build/build.xml

Issues I can imagine are if someone decides to rename style.css or script.js and expects the build script to continue swapping for concatenated files.

Should add two more values to default.properties which specify the names of these files?

edit: 6a77f5ad7e996c539c87b1b18ef74ee44bef45bc (includes property for script.js in default.properties)

kevva commented 12 years ago

ahh, very nice. works as expected!

drublic commented 12 years ago

Works very nice for me, too.

@darktable, could you please post a pull request? If you want I can do it for you. Let me know.

darktable commented 12 years ago

pull request #705.

paulirish commented 12 years ago

boom. closed in #705 and 7467f9c0417a0c1f9863e2d000aad73f34836ef2

:D