[webkit-reviews] review granted: [Bug 119423] Improve srcset parser : [Attachment 211535] Patch

[bugzilla-daemon at webkit.org]

Benjamin Poulain <benjamin at webkit.org> has granted Romain Perier
<romain.perier at gmail.com>'s request for review:
Bug 119423: Improve srcset parser
https://bugs.webkit.org/show_bug.cgi?id=119423

Attachment 211535: Patch
https://bugs.webkit.org/attachment.cgi?id=211535&action=review

------- Additional Comments from Benjamin Poulain <benjamin at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=211535&action=review


This looks great. Some minor comments bellow.

Can you please also add tests for a srcset descriptor with spaces around (if
there is no such test yet). Something like srcset="    foobar.jpg",
srcset="fobar.jpg    " and srcset="   foobar.jpg   ". (+ the variations with a
scale factor if needed).

> Source/WebCore/html/parser/HTMLParserIdioms.cpp:361
> +		   size_t comma = imageScaleEnd;

comma -> commaPosition

> Source/WebCore/html/parser/HTMLParserIdioms.cpp:362
> +		   // Make sure there are no other descriptors

Missing period.

> Source/WebCore/html/parser/HTMLParserIdioms.cpp:391
> +	   image.imageURL =
decodeURLEscapeSequences(StringImpl::createWithoutCopying(srcSetAttribute.chara
cters() + imageUrlStart, imageUrlEnd - imageUrlStart));

Please check decodeURLEscapeSequences and do a follow up if the current
behavior is erroneous.