search

jrit / web-resource-inliner

Inlines img, script and link tags into the same file
MIT License
69 stars 30 forks source link

Identical URLs in CSS File Rebase Incorrectly #2

Closed tooolbox closed 8 years ago

tooolbox commented 9 years ago

In css.js, there is a loop:

while( ( found = urlRegex.exec( result ) ) !== null )
    {
        if( !found[ 0 ].match( new RegExp( "\\/\\*\\s?" + settings.inlineAttribute + "-ignore\\s?\\*\\/", "gi" ) )
            && ( settings.images || found[ 0 ].match( new RegExp( "\\/\\*\\s?" + settings.inlineAttribute + "\\s?\\*\\/", "gi" ) ) ) )
        {
            tasks.push( replaceUrl.bind(
            {
                src: found[ 1 ],
                limit: settings.images
            } ) );
        }
    }

Because your are doing regex replaces (rebasing) at the same time that you are finding url() references, and your regex replaces a global, if there are two identical url() references, the second one will not be rebased correctly.

For example:

url("../img/blah.png")
url("../img/blah.png")

Rebased to xyz would create:

url("img/blah.png")
url("xyz/img/blah.png")

Perhaps a better way to do this would be to loop through the document and find matches, assign those to an array to de-dup them, and then do a global regexp replace for each one in sequence.

I'm not saying this is a typical situation, but I did run into it. In case you're wondering what I'm doing, I'm using Juice (which uses this package) to do automatic processing of ebook files that were generated by InDesign. The CSS that ID generates is rather repetitive and includes multiple identical url() references, and the rebasing behavior of web-resource-inliner was making Juice explode on me.

jrit commented 9 years ago

@tooolbox I agree about the proposed solution. There are some related optimizations that should be done to 1) only ever try to retrieve a resource once and 2) technically, someone could want to only replace one reference and not another, but right now if one instance matches than all of the same matches are also replaced. If you have an opportunity to create a PR to fix this that would be awesome.