wip: prepend is not applied to all urls when used with a srcset

[st-h]

This is another (ongoing) attempt to finally fix the issues with srcset, which leads to images being loaded from non existent paths or domains when the prepend option is used.

This PR adds failing test cases for scenarios that currently do not work as they should: In a srcset only the first path is prepended, while all occurrences seem to be replaced with the correct substitute.

[st-h]

The last commit should fix the issue. I've taken the solution form the PR: https://github.com/ember-cli/broccoli-asset-rewrite/pull/56/files which seems to work as expected. My Regex knowledge is a bit rusty at the moment, so please let me know in case you find any issues with that approach.

[st-h]

Turns out this approach still has issues when the srcset includes a newline like:

srcset="/assets/img/other-small.png 800w, 
        /assets/img/other-medium.png 1600w"

which produces the following output, that is rejected by browsers:

srcset="https://prepend.com/assets/img/other-small.png 800w, https://prepend.com/
        /assets/img/other-medium.png 1600w"

A trivial solution might be to just remove the newlines within the string. Yet, there might be better approaches, so I appreciate any feedback on this. I am still not very confident with my understandings about the intentions of the existing code.

Yet, the current state of this PR allows to work around the srcset limitations of the currently used versions within ember apps.

[GCheung55]

Turns out this approach still has issues when the srcset includes a newline like:

srcset="/assets/img/other-small.png 800w, 
        /assets/img/other-medium.png 1600w"

Would removing the linebreaks address the issue?

[GCheung55]

@st-h how about adding a test for the line break issue?

[st-h]

Yes. Good call. Just haven't had any time to look into this further.

[st-h]

Turns out this approach still has issues when the srcset includes a newline like:

srcset="/assets/img/other-small.png 800w, 
        /assets/img/other-medium.png 1600w"

Would removing the linebreaks address the issue?

Yes, I am currently using this fork with our beta version, without linebreaks in srcsets. Didn't came across any issues so far.

[crixx]

@st-h @GCheung55 any thing I can help with to get this merged asap?

edit: just found another PR for the same issue: #61

[st-h]

@crixx any sort of improvement is very welcome 🙃 At very least we need to fix the newline issue. Maybe there is a better approach? Otherwise, it would be great if someone could review that regex (didn't have time yet to fully figure it out).

I am positive we then can get this merged in a reasonable amount of time. Usually needs some persistence though 😂

I also saw the other PR, but I had some issues identifying the reasonable changes and how they would fit into the current codebase.

[crixx]

@st-h if we use a 3rd image as input, also the enhanced regex does not match anymore. The third image is not prepended.

const output = createBuilder(subject);
    input.write({
      'srcset.html': `<source srcset="/assets/img/small.png 800w, /assets/img/medium.png 1600w, /assets/img/large.png 2000w" sizes="(max-width: 800px) 800px, (max-width: 1600px) 1600px, 2000px" type="image/png">`,
    });

    await output.build();
    expect(output.changes()).to.deep.equal({
      'srcset.html': 'create',
    });
    expect(output.read()).to.deep.equal({
      'srcset.html': `<source srcset="my-path/assets/img/other-small.png 800w, my-path/assets/img/other-medium.png 1600w,  /assets/img/other-large.png 2000w" sizes="(max-width: 800px) 800px, (max-width: 1600px) 1600px, 2000px" type="image/png">`,
    });

returns

"srcset.html": "<source srcset=\"my-path/assets/img/other-small.png 800w, my-path/assets/img/other-medium.png 1600w, /assets/img/large.png 2000w\" sizes=\"(max-width: 800px) 800px, (max-width: 1600px) 1600px, 2000px\" type=\"image/png\">"

Regarding the linebreak: adding an input like:

input.write({
      'srcset.html': `<source srcset="/assets/img/small.png 1x,
                        /assets/img/medium.png 2x,
                        /assets/img/large.png 3x"
                        type="image/png">`,
    });

returns an output like this:

 "srcset.html": "<source srcset=\"https://subdomain.cloudfront.net/assets/img/other-small.png 1x,\n                        https://subdomain.cloudfront.net/assets/img/other-medium.png 2x,\n                        /assets/img/large.png 3x\"\n                        type=\"image/png\">"

So, at least on my Mac the linebreak does look good, but the last image is not prefixed

[esbanarango]

This PR fixes: #69.