Skip to content

GitLab

T
tangled
Commit c56532c7 authored by yoav@yoav.ws's avatar yoav@yoav.ws
Browse files
Options

Fix two srcset related bugs 

Two problems with the current srcset parsing algorithm are fixed here:
1. When a set of descriptors contained only invalid descriptors, the resource wasn't ignored.
2. When more than 1 resource shared a single DPR value, the last one was taken into account, rather than the first one.

I've also added test and fixed current ones to prevent this from regressing in the future.

BUG=347998

Review URL: https://codereview.chromium.org/184523003

git-svn-id: svn://svn.chromium.org/blink/trunk@168241 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent 06b715a8
Hide whitespace changes
Inline Side-by-side
Showing with 54 additions and 13 deletions
third_party/WebKit/LayoutTests/fast/hidpi/image-srcset-invalid-descriptor-expected.txt
View file @ c56532c7
......@@ -2,6 +2,7 @@ PASS successfullyParsed is true
TEST COMPLETE
PASS document.getElementById("foo").clientWidth==100 is true
PASS document.getElementById("foo2").clientWidth==200 is true
This test passes if the image below is a 100px blue square when the deviceScaleFactor is 1.
It tests that even though the 1x resource contains many invalid descriptors, only the invalid descriptors are ignored (according to the spec)
It tests that even though the 1x resource contains many invalid descriptors, only the invalid descriptors are ignored (according to the spec).
third_party/WebKit/LayoutTests/fast/hidpi/image-srcset-invalid-descriptor.html
View file @ c56532c7
......@@ -8,14 +8,16 @@
<script>
addEventListener("load", function() {
shouldBeTrue('document.getElementById("foo").clientWidth==100');
shouldBeTrue('document.getElementById("foo2").clientWidth==200');
}, false);
</script>
</head>
<body id="body">
<div>This test passes if the image below is a 100px blue square when the deviceScaleFactor is 1.</div>
<div> It tests that even though the 1x resource contains many invalid descriptors, only the invalid descriptors are ignored (according
to the spec) </div>
<div> It tests that even though the 1x resource contains many invalid descriptors,
only the invalid descriptors are ignored (according to the spec).</div>
<img id="foo" src="" srcset="resources/blue-100-px-square.png 43q 1x dfs 3dd, resources/green-400-px-square.png 2x">
<img id="foo2" src="" srcset="resources/blue-100-px-square.png 300q, resources/green-400-px-square.png 2x">
</body>
</html>
third_party/WebKit/LayoutTests/fast/hidpi/image-srcset-invalid-inputs-correct-src.html
View file @ c56532c7
......@@ -16,6 +16,6 @@
<body id="body">
<div>This test passes if this img tag below is a green square regardless of the scale factor. It ensures that invalid inputs
from srcset are ignored, and src is selected (since it's the only candidate left)</div>
<img id="foo" src="resources/green-400-px-square.png" srcset="1x,, , , resources/blue-100-px-square.png 2px, 2x ,,">
<img id="foo" src="resources/green-400-px-square.png" srcset="1x 5w, , , resources/blue-100-px-square.png 2px, 2x 3h,,">
</body>
</html>
third_party/WebKit/LayoutTests/fast/hidpi/image-srcset-src-selection-1x-both-expected.txt 0 → 100644
View file @ c56532c7
PASS successfullyParsed is true
TEST COMPLETE
PASS document.getElementById("foo").clientWidth==400 is true
This test passes if the div below is a blue 100px square when the deviceScaleFactor is 1. It simply ensures that when both src and srcset are specified for the same DPR, srcset wins.
third_party/WebKit/LayoutTests/fast/hidpi/image-srcset-src-selection-1x-both.html 0 → 100644
View file @ c56532c7
<html>
<head>
<script>
window.targetScaleFactor = 2;
</script>
<script src="resources/srcset-helper.js"></script>
<script src="../../resources/js-test.js"></script>
<script>
if (window.testRunner) {
testRunner.dumpAsText();
}
addEventListener("load", function() {
shouldBeTrue('document.getElementById("foo").clientWidth==400');
}, false);
</script>
</head>
<body id="body">
<div>This test passes if the div below is a blue 100px square when the deviceScaleFactor is 1.
It simply ensures that when both src and srcset are specified for the same DPR, srcset wins.</div>
<img id="foo" src="resources/blue-100-px-square.png" srcset="resources/green-400-px-square.png 1x">
</body>
</html>
third_party/WebKit/Source/core/html/parser/HTMLSrcsetParser.cpp
View file @ c56532c7
......@@ -51,8 +51,9 @@ template<typename CharType>
static bool parseDescriptors(const CharType* descriptorsStart, const CharType* descriptorsEnd, float& imgScaleFactor)
{
const CharType* position = descriptorsStart;
bool isValid = true;
bool isScaleFactorFound = false;
bool isValid = false;
bool isFoundScaleFactor = false;
bool isEmptyDescriptor = !(descriptorsEnd > descriptorsStart);
while (position < descriptorsEnd) {
// 13.1. Let descriptor list be the result of splitting unparsed descriptors on spaces.
skipWhile<CharType, isHTMLSpace<CharType> >(position, descriptorsEnd);
......@@ -65,15 +66,15 @@ static bool parseDescriptors(const CharType* descriptorsStart, const CharType* d
--currentDescriptorEnd;
unsigned descriptorLength = currentDescriptorEnd - currentDescriptorStart;
if (*currentDescriptorEnd == 'x') {
if (isScaleFactorFound)
if (isFoundScaleFactor)
return false;
imgScaleFactor = charactersToFloat(currentDescriptorStart, descriptorLength, &isValid);
isScaleFactorFound = true;
isFoundScaleFactor = true;
} else {
continue;
}
}
return isValid;
return isEmptyDescriptor || isValid;
}
// http://www.whatwg.org/specs/web-apps/current-work/multipage/embedded-content-1.html#processing-the-image-candidates
......@@ -138,11 +139,18 @@ static ImageCandidate pickBestImageCandidate(float deviceScaleFactor, Vector<Ima
std::stable_sort(imageCandidates.begin(), imageCandidates.end(), compareByScaleFactor);
unsigned i;
for (i = 0; i < imageCandidates.size() - 1; ++i) {
for (i = 0; i < imageCandidates.size() - 1; ++i)
if (imageCandidates[i].scaleFactor() >= deviceScaleFactor)
break;
}
return imageCandidates[i];
float winningScaleFactor = imageCandidates[i].scaleFactor();
unsigned winner = i;
// 16. If an entry b in candidates has the same associated ... pixel density as an earlier entry a in candidates,
// then remove entry b
while ((i > 0) && (imageCandidates[--i].scaleFactor() == winningScaleFactor))
winner = i;
return imageCandidates[winner];
}
ImageCandidate bestFitSourceForSrcsetAttribute(float deviceScaleFactor, const String& srcsetAttribute)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment