Bug 161543 - Align meta element http-equiv="refresh" parsing with the HTML specification
Summary: Align meta element http-equiv="refresh" 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/multipag...
Keywords: WebExposed
Depends on:
Blocks:
 
Reported: 2016-09-02 12:39 PDT by Chris Dumez
Modified: 2017-08-03 19:45 PDT (History)
10 users (show)

See Also:


Attachments
Patch (28.56 KB, patch)
2016-09-02 13:45 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-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.