Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Identical URLs in CSS File Rebase Incorrectly #2

Closed
tooolbox opened this issue Mar 14, 2015 · 1 comment
Closed

Identical URLs in CSS File Rebase Incorrectly #2

tooolbox opened this issue Mar 14, 2015 · 1 comment

Comments

@tooolbox
Copy link
Contributor

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
Copy link
Owner

jrit commented Mar 28, 2015

@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.

tooolbox added a commit to tooolbox/web-resource-inliner that referenced this issue May 19, 2016
@jrit jrit closed this as completed in #20 May 20, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants