Bug 161543

Summary: Align meta element http-equiv="refresh" parsing with the HTML specification
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: DOMAssignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, commit-queue, darin, dbates, esprehn+autocc, gyuyoung.kim, japhet, kangil.han, rniwa, sam
Priority: P2 Keywords: WebExposed
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
URL: https://html.spec.whatwg.org/multipage/semantics.html#attr-meta-http-equiv-refresh
See Also: https://bugs.webkit.org/show_bug.cgi?id=175132
Attachments:
Description Flags
Patch none

Description Chris Dumez 2016-09-02 12:39:00 PDT
Align meta element http-equiv="refresh" parsing with the HTML specification:
- https://html.spec.whatwg.org/multipage/semantics.html#attr-meta-http-equiv-refresh
Comment 1 Chris Dumez 2016-09-02 13:45:14 PDT
Created attachment 287809 [details]
Patch
Comment 2 Darin Adler 2016-09-03 06:49:11 PDT
Comment on attachment 287809 [details]
Patch

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

> Source/WebCore/html/parser/HTMLParserIdioms.cpp:305
> +    Optional<int> number = parseHTMLNonNegativeInteger(StringView(numberStart, position - numberStart).toStringWithoutCopying());

We need to change parseHTMLNonNegativeInteger to take a StringView; it’s terrible to allocate memory to do this operation!

I particularly thing it’s peculiar to make this entire function a template based on character type, which we should only do if the code is really performance-sensitive, and then allocate memory just to parse an integer!

> Source/WebCore/platform/network/HTTPParsers.h:81
> -bool parseHTTPRefresh(const String& refresh, bool fromHttpEquivMeta, double& delay, String& url);
> +bool parseHTTPRefresh(const String& refresh, double& delay, String& url);

I don’t understand why two separate parsers is the way to go here. Do we really need both?
Comment 3 WebKit Commit Bot 2016-09-03 07:11:54 PDT
Comment on attachment 287809 [details]
Patch

Clearing flags on attachment: 287809

Committed r205400: <http://trac.webkit.org/changeset/205400>
Comment 4 WebKit Commit Bot 2016-09-03 07:12:01 PDT
All reviewed patches have been landed.  Closing bug.