Bug 61759 - [Chromium] Make isValidProtocol() accept protocols with '+'.
Summary: [Chromium] Make isValidProtocol() accept protocols with '+'.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: James Kozianski
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-05-30 21:32 PDT by James Kozianski
Modified: 2011-05-30 23:29 PDT (History)
1 user (show)

See Also:

Attachments
Patch (5.90 KB, patch)
2011-05-30 21:38 PDT, James Kozianski 		no flags Details | Formatted Diff | Diff
Patch (5.68 KB, patch)
2011-05-30 22:25 PDT, James Kozianski 		no flags Details | Formatted Diff | Diff
Patch (5.67 KB, patch)
2011-05-30 22:46 PDT, James Kozianski 		no flags Details | Formatted Diff | Diff
Patch (5.67 KB, patch)
2011-05-30 22:56 PDT, James Kozianski 		tkent: review+
Details | Formatted Diff | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description James Kozianski 2011-05-30 21:32:33 PDT 
[Chromium] Update registerProtocolHandler tests for chromium.
Comment 1 James Kozianski 2011-05-30 21:38:10 PDT 
Created attachment 95393 [details]
Patch
Comment 2 James Kozianski 2011-05-30 22:25:28 PDT 
Created attachment 95395 [details]
Patch
Comment 3 Kent Tamura 2011-05-30 22:38:45 PDT 
Comment on attachment 95395 [details]
Patch

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

> Source/WebCore/ChangeLog:10
> +        Detect syntax errors before security errors.

Could you write reasons why we need to detect syntax errors earlier?

> Source/WebCore/ChangeLog:15
> +2011-05-30  James Kozianski  <koz@chromium.org>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        [Chromium] Make isValidProtocol() accept protocols with '+'.
> +        https://bugs.webkit.org/show_bug.cgi?id=61759
> +
> +        * page/Navigator.cpp:
> +        (WebCore::Navigator::registerProtocolHandler):
> +        Detect syntax errors before security errors.
> +        * platform/KURLGoogle.cpp:
> +        (WebCore::isSchemeChar):
> +        Include '+' in the list of valid characters.
> +
> +2011-05-30  James Kozianski  <koz@chromium.org>

Teere are two ChangeLog entries.
Comment 4 James Kozianski 2011-05-30 22:46:40 PDT 
Created attachment 95396 [details]
Patch
Comment 5 James Kozianski 2011-05-30 22:47:47 PDT 
Comment on attachment 95395 [details]
Patch

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

>> Source/WebCore/ChangeLog:10
>> +        Detect syntax errors before security errors.
> 
> Could you write reasons why we need to detect syntax errors earlier?

Done.

>> Source/WebCore/ChangeLog:15
>> +2011-05-30  James Kozianski  <koz@chromium.org>
> 
> Teere are two ChangeLog entries.

Done.
Comment 6 Kent Tamura 2011-05-30 22:54:42 PDT 
Comment on attachment 95396 [details]
Patch

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

> Source/WebCore/ChangeLog:11
> +        [Chromium] Make isValidProtocol() accept protocols with '+'.
> +
> +        Also, detect syntax errors before security errors; some syntax errors
> +        will also trigger a security error, but it's more helpful to the
> +        programmer to know if they have a syntax error, which are well-defined
> +        in the spec, rather than a security error, which aren't.
> +        https://bugs.webkit.org/show_bug.cgi?id=61759

nit: Usually we use the order of 
 <One-line summary>
 <Bug URL>

 <Description>
Comment 7 James Kozianski 2011-05-30 22:56:47 PDT 
Created attachment 95397 [details]
Patch
Comment 8 James Kozianski 2011-05-30 22:57:32 PDT 
Comment on attachment 95396 [details]
Patch

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

>> Source/WebCore/ChangeLog:11
>> +        https://bugs.webkit.org/show_bug.cgi?id=61759
> 
> nit: Usually we use the order of 
>  <One-line summary>
>  <Bug URL>
> 
>  <Description>

Done.
Comment 9 James Kozianski 2011-05-30 23:29:39 PDT 
Committed r87713: <http://trac.webkit.org/changeset/87713>