Bug 161636 - Align srcset attribute parsing with the HTML specification
Summary: Align srcset attribute parsing with the HTML specification
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL: https://html.spec.whatwg.org/#parse-a...
Keywords: WebExposed
Depends on:
Blocks:
 
Reported: 2016-09-06 12:04 PDT by Chris Dumez
Modified: 2016-09-06 16:01 PDT (History)
6 users (show)

See Also:


Attachments
Patch (33.55 KB, patch)
2016-09-06 12:37 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2016-09-06 12:04:19 PDT
Align srcset attribute parsing with the HTML specification:
- https://html.spec.whatwg.org/#parse-a-srcset-attribute

This is covered by:
http://w3c-test.org/html/semantics/embedded-content/the-img-element/srcset/parse-a-srcset-attribute.html

Firefox and Chrome pass all of these checks. However, WebKit fails 100 out of 236.
Comment 1 Chris Dumez 2016-09-06 12:37:04 PDT
Created attachment 288037 [details]
Patch
Comment 2 Darin Adler 2016-09-06 15:39:24 PDT
Comment on attachment 288037 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=288037&action=review

> Source/WebCore/html/parser/HTMLSrcsetParser.cpp:137
> +            Optional<double> density = parseValidHTMLFloatingPointNumber(descriptor);

auto?

> Source/WebCore/html/parser/HTMLSrcsetParser.cpp:144
> +            Optional<int> resourceWidth = parseValidHTMLNonNegativeInteger(descriptor);

auto?

> Source/WebCore/html/parser/HTMLSrcsetParser.cpp:153
> +            Optional<int> resourceHeight = parseValidHTMLNonNegativeInteger(descriptor);

auto?
Comment 3 Chris Dumez 2016-09-06 15:41:36 PDT
Comment on attachment 288037 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=288037&action=review

>> Source/WebCore/html/parser/HTMLSrcsetParser.cpp:137
>> +            Optional<double> density = parseValidHTMLFloatingPointNumber(descriptor);
> 
> auto?

Optional<> is one of the only cases where I personally prefer not to use auto, for clarity because the !density check below becomes a bit confusing.
Comment 4 WebKit Commit Bot 2016-09-06 16:01:28 PDT
Comment on attachment 288037 [details]
Patch

Clearing flags on attachment: 288037

Committed r205515: <http://trac.webkit.org/changeset/205515>
Comment 5 WebKit Commit Bot 2016-09-06 16:01:32 PDT
All reviewed patches have been landed.  Closing bug.