Commit cfac132a authored by yoav@yoav.ws's avatar yoav@yoav.ws

Align srcset parsing with spec changed

This CL align the srcset parsing algorithm with recent picture spec changes.
Specifically:
* Src URL are now not part of the source selection algoritm, as per https://github.com/ResponsiveImagesCG/picture-element/pull/147
* 0w resources are ignored, to avoid 0/0 operations, and because they make no sense, as per https://github.com/ResponsiveImagesCG/picture-element/pull/151

BUG=357586

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

git-svn-id: svn://svn.chromium.org/blink/trunk@171489 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent ec878a9f
...@@ -75,7 +75,7 @@ static bool parseDescriptors(const CharType* descriptorsStart, const CharType* d ...@@ -75,7 +75,7 @@ static bool parseDescriptors(const CharType* descriptorsStart, const CharType* d
if (result.foundDescriptor()) if (result.foundDescriptor())
return false; return false;
result.resourceWidth = charactersToInt(currentDescriptorStart, descriptorLength, &isValid); result.resourceWidth = charactersToInt(currentDescriptorStart, descriptorLength, &isValid);
if (!isValid || result.resourceWidth < 0) if (!isValid || result.resourceWidth <= 0)
return false; return false;
} }
} }
...@@ -122,7 +122,11 @@ static void parseImageCandidatesFromSrcsetAttribute(const String& attribute, con ...@@ -122,7 +122,11 @@ static void parseImageCandidatesFromSrcsetAttribute(const String& attribute, con
continue; continue;
} }
imageCandidates.append(ImageCandidate(attribute, imageURLStart - attributeStart, imageURLEnd - imageURLStart, result)); ASSERT(imageURLEnd > attributeStart);
unsigned imageURLStartingPosition = imageURLStart - attributeStart;
ASSERT(imageURLEnd > imageURLStart);
unsigned imageURLLength = imageURLEnd - imageURLStart;
imageCandidates.append(ImageCandidate(attribute, imageURLStartingPosition, imageURLLength, result, ImageCandidate::SrcsetOrigin));
// 11. Return to the step labeled splitting loop. // 11. Return to the step labeled splitting loop.
} }
} }
...@@ -140,20 +144,23 @@ static void parseImageCandidatesFromSrcsetAttribute(const String& attribute, Vec ...@@ -140,20 +144,23 @@ static void parseImageCandidatesFromSrcsetAttribute(const String& attribute, Vec
static ImageCandidate pickBestImageCandidate(float deviceScaleFactor, int effectiveSize, Vector<ImageCandidate>& imageCandidates) static ImageCandidate pickBestImageCandidate(float deviceScaleFactor, int effectiveSize, Vector<ImageCandidate>& imageCandidates)
{ {
bool ignoreSrc = false;
if (imageCandidates.isEmpty()) if (imageCandidates.isEmpty())
return ImageCandidate(); return ImageCandidate();
// http://picture.responsiveimages.org/#normalize-source-densities // http://picture.responsiveimages.org/#normalize-source-densities
for (Vector<ImageCandidate>::iterator it = imageCandidates.begin(); it != imageCandidates.end(); ++it) { for (Vector<ImageCandidate>::iterator it = imageCandidates.begin(); it != imageCandidates.end(); ++it) {
if (it->scaleFactor() < 0) if (it->resourceWidth() > 0) {
it->setScaleFactor((float)it->resourceWidth() / (float)effectiveSize); it->setScaleFactor((float)it->resourceWidth() / (float)effectiveSize);
ignoreSrc = true;
}
} }
std::stable_sort(imageCandidates.begin(), imageCandidates.end(), compareByScaleFactor); std::stable_sort(imageCandidates.begin(), imageCandidates.end(), compareByScaleFactor);
unsigned i; unsigned i;
for (i = 0; i < imageCandidates.size() - 1; ++i) { for (i = 0; i < imageCandidates.size() - 1; ++i) {
if (imageCandidates[i].scaleFactor() >= deviceScaleFactor) if ((imageCandidates[i].scaleFactor() >= deviceScaleFactor) && (!ignoreSrc || !imageCandidates[i].srcOrigin()))
break; break;
} }
...@@ -184,7 +191,7 @@ ImageCandidate bestFitSourceForImageAttributes(float deviceScaleFactor, int effe ...@@ -184,7 +191,7 @@ ImageCandidate bestFitSourceForImageAttributes(float deviceScaleFactor, int effe
if (srcsetAttribute.isNull()) { if (srcsetAttribute.isNull()) {
if (srcAttribute.isNull()) if (srcAttribute.isNull())
return ImageCandidate(); return ImageCandidate();
return ImageCandidate(srcAttribute, 0, srcAttribute.length(), defaultResult); return ImageCandidate(srcAttribute, 0, srcAttribute.length(), defaultResult, ImageCandidate::SrcOrigin);
} }
Vector<ImageCandidate> imageCandidates; Vector<ImageCandidate> imageCandidates;
...@@ -192,7 +199,7 @@ ImageCandidate bestFitSourceForImageAttributes(float deviceScaleFactor, int effe ...@@ -192,7 +199,7 @@ ImageCandidate bestFitSourceForImageAttributes(float deviceScaleFactor, int effe
parseImageCandidatesFromSrcsetAttribute(srcsetAttribute, imageCandidates); parseImageCandidatesFromSrcsetAttribute(srcsetAttribute, imageCandidates);
if (!srcAttribute.isEmpty()) if (!srcAttribute.isEmpty())
imageCandidates.append(ImageCandidate(srcAttribute, 0, srcAttribute.length(), defaultResult)); imageCandidates.append(ImageCandidate(srcAttribute, 0, srcAttribute.length(), defaultResult, ImageCandidate::SrcOrigin));
return pickBestImageCandidate(deviceScaleFactor, effectiveSize, imageCandidates); return pickBestImageCandidate(deviceScaleFactor, effectiveSize, imageCandidates);
} }
...@@ -209,7 +216,7 @@ String bestFitSourceForImageAttributes(float deviceScaleFactor, int effectiveSiz ...@@ -209,7 +216,7 @@ String bestFitSourceForImageAttributes(float deviceScaleFactor, int effectiveSiz
imageCandidates.append(srcsetImageCandidate); imageCandidates.append(srcsetImageCandidate);
if (!srcAttribute.isEmpty()) if (!srcAttribute.isEmpty())
imageCandidates.append(ImageCandidate(srcAttribute, 0, srcAttribute.length(), defaultResult)); imageCandidates.append(ImageCandidate(srcAttribute, 0, srcAttribute.length(), defaultResult, ImageCandidate::SrcOrigin));
return pickBestImageCandidate(deviceScaleFactor, effectiveSize, imageCandidates).toString(); return pickBestImageCandidate(deviceScaleFactor, effectiveSize, imageCandidates).toString();
} }
......
...@@ -53,15 +53,21 @@ struct DescriptorParsingResult { ...@@ -53,15 +53,21 @@ struct DescriptorParsingResult {
class ImageCandidate { class ImageCandidate {
public: public:
enum OriginAttribute {
SrcsetOrigin,
SrcOrigin
};
ImageCandidate() ImageCandidate()
: m_scaleFactor(1.0) : m_scaleFactor(1.0)
{ {
} }
ImageCandidate(const String& source, unsigned start, unsigned length, const DescriptorParsingResult& result) ImageCandidate(const String& source, unsigned start, unsigned length, const DescriptorParsingResult& result, OriginAttribute originAttribute)
: m_string(source.createView(start, length)) : m_string(source.createView(start, length))
, m_scaleFactor(result.scaleFactor) , m_scaleFactor(result.scaleFactor)
, m_resourceWidth(result.resourceWidth) , m_resourceWidth(result.resourceWidth)
, m_originAttribute(originAttribute)
{ {
} }
...@@ -90,6 +96,11 @@ public: ...@@ -90,6 +96,11 @@ public:
return m_resourceWidth; return m_resourceWidth;
} }
bool srcOrigin() const
{
return (m_originAttribute == SrcOrigin);
}
inline bool isEmpty() const inline bool isEmpty() const
{ {
return m_string.isEmpty(); return m_string.isEmpty();
...@@ -99,6 +110,7 @@ private: ...@@ -99,6 +110,7 @@ private:
StringView m_string; StringView m_string;
float m_scaleFactor; float m_scaleFactor;
int m_resourceWidth; int m_resourceWidth;
OriginAttribute m_originAttribute;
}; };
ImageCandidate bestFitSourceForSrcsetAttribute(float deviceScaleFactor, int effectiveSize, const String& srcsetAttribute); ImageCandidate bestFitSourceForSrcsetAttribute(float deviceScaleFactor, int effectiveSize, const String& srcsetAttribute);
......
...@@ -46,6 +46,12 @@ TEST(HTMLSrcsetParserTest, Basic) ...@@ -46,6 +46,12 @@ TEST(HTMLSrcsetParserTest, Basic)
{5.0, -1, "", "1x,, , x ,2x , 1x.gif, 3x, 4x.gif 4x 100h, 5x.gif 5, dx.gif dx, 2x.gif 2x ,", "4x.gif", 4.0, -1}, {5.0, -1, "", "1x,, , x ,2x , 1x.gif, 3x, 4x.gif 4x 100h, 5x.gif 5, dx.gif dx, 2x.gif 2x ,", "4x.gif", 4.0, -1},
{1.0, 400, "", "400.gif 400w, 6000.gif 6000w", "400.gif", 1.0, 400}, {1.0, 400, "", "400.gif 400w, 6000.gif 6000w", "400.gif", 1.0, 400},
{2.0, 400, "", "400.gif 400w, 6000.gif 6000w", "6000.gif", 15.0, 6000}, {2.0, 400, "", "400.gif 400w, 6000.gif 6000w", "6000.gif", 15.0, 6000},
{1.0, 400, "src.gif", "800.gif 800w", "800.gif", 2.0, 800},
{1.0, 400, "src.gif", "0.gif 0w, 800.gif 800w", "800.gif", 2.0, 800},
{1.0, 400, "src.gif", "0.gif 0w, 2x.gif 2x", "src.gif", 1.0, -1},
{1.0, 400, "src.gif", "800.gif 2x, 1600.gif 1600w", "800.gif", 2.0, -1},
{1.0, 400, "", "400.gif 400w, 2x.gif 2x", "400.gif", 1.0, 400},
{2.0, 400, "", "400.gif 400w, 2x.gif 2x", "2x.gif", 2.0, -1},
{1.0, 0, "", "400.gif 400w, 6000.gif 6000w", "400.gif", std::numeric_limits<float>::infinity(), 400}, {1.0, 0, "", "400.gif 400w, 6000.gif 6000w", "400.gif", std::numeric_limits<float>::infinity(), 400},
{0, 0, 0, 0, 0, 0} // Do not remove the terminator line. {0, 0, 0, 0, 0, 0} // Do not remove the terminator line.
}; };
......
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