Skip to content
This repository has been archived by the owner on Jun 11, 2024. It is now read-only.

Commit

Permalink
Fix grabbing urls from srcset attributes
Browse files Browse the repository at this point in the history
My first attempt at this (312ecd9) was too naive, it didn't take urls
with commas in them into account. In this second attempt, I've tried to
tdd things so that I know we can handle a wider range of values.

I've used https://developer.mozilla.org/en-US/docs/Web/HTML/Element/img
as a guide into figuring out possible values.
  • Loading branch information
trotzig committed Jun 10, 2018
1 parent 6f2d162 commit dc21c33
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 4 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import getUrlsFromSrcset from '../getUrlsFromSrcset';

it('handles invalid values', () => {
expect(getUrlsFromSrcset(' ')).toEqual([]);
});

it('handles single urls without descriptor', () => {
expect(getUrlsFromSrcset('/foo.png')).toEqual(['/foo.png']);
});

it('handles single urls with commas', () => {
expect(getUrlsFromSrcset('/foo,40,50.png')).toEqual(['/foo,40,50.png']);
});

it('handles single urls with a width descriptor', () => {
expect(getUrlsFromSrcset('http://foo.com/foo.png 200w')).toEqual(['http://foo.com/foo.png']);
});

it('handles single urls with a pixel density descriptor', () => {
expect(getUrlsFromSrcset('http://foo.com/foo.png 1.2x')).toEqual(['http://foo.com/foo.png']);
});

it('strips whitespace', () => {
expect(getUrlsFromSrcset(`
/foo.png`)).toEqual(['/foo.png']);
});

it('handles multiple urls', () => {
expect(getUrlsFromSrcset('/foo.png 200w,/bar.png 400w')).toEqual(['/foo.png', '/bar.png']);
});

it('handles multiple urls with commas', () => {
expect(getUrlsFromSrcset('http://foo.com/foo,40,50 200w,/bar,20.png 400w')).toEqual(['http://foo.com/foo,40,50', '/bar,20.png']);
});

it('handles multiple urls with whitespace', () => {
expect(getUrlsFromSrcset(`
http://foo.com/foo,40,50 200w,
/bar,20.png 400w
`)).toEqual(['http://foo.com/foo,40,50', '/bar,20.png']);
});
10 changes: 10 additions & 0 deletions packages/happo-target-firefox/src/getUrlsFromSrcset.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
const SRCSET_ITEM = /([^\s]+)(\s+[0-9.]+[wx])?(,?\s*)/g;

export default function getUrlsFromSrcset(value) {
const result = [];
let match;
while (match = SRCSET_ITEM.exec(value)) { // eslint-disable-line no-cond-assign
result.push(match[1]);
}
return result;
}
6 changes: 2 additions & 4 deletions packages/happo-target-firefox/src/waitForImagesToRender.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import findBackgroundImageUrls from './findBackgroundImageUrls';
import getUrlsFromSrcset from './getUrlsFromSrcset';

export function waitForImageToLoad(url) {
return new Promise((resolve, reject) => {
Expand All @@ -23,10 +24,7 @@ export default function waitForImagesToRender() {
return;
}

srcset.split(/,\s*/).forEach((entry) => {
const [url] = entry.split(' ');
promises.push(waitForImageToLoad(url));
});
promises.push(...getUrlsFromSrcset(srcset).map(waitForImageToLoad));
});

Array.prototype.slice.call(document.body.querySelectorAll('*'))
Expand Down

0 comments on commit dc21c33

Please sign in to comment.