[Webkit-unassigned] [Bug 52609] Add navigator.registerProtocolHandler behind a flag.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=52609


Ojan Vafai <ojan at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ojan at chromium.org




--- Comment #28 from Ojan Vafai <ojan at chromium.org>  2011-01-30 18:44:37 PST ---
View in context: https://bugs.webkit.org/attachment.cgi?id=80604&action=review

Few more nits in addition to dimitri's.

> LayoutTests/fast/dom/registerProtocolHandler.html:22
> +// Test that the API regects secured protocols.

s/regects/rejects. also, this comment is kind of useless. it's pretty clear from the variable name what's going on.

> LayoutTests/fast/dom/registerProtocolHandler.html:31
> +        if('SECURITY_ERR' == e.name) {
> +            succeeded = true;
> +        };

no brackets on one-line ifs. and no semicolon after if-statement curly braces. also, you could just write this as:
succeeded = 'SECURITY_ERR' == e.name;

> LayoutTests/fast/dom/registerProtocolHandler.html:38
> +    if(succeeded == true) {
> +        debug('Pass: Invalid protocol "' + protocol + '" threw SECURITY_ERR exception.');
> +    } else {
> +        debug('Fail: Invalid protocol "' + protocol + '" allowed.');
> +    };

nit: webkit style is to leave out the curly braces. also, there shouldn't be a semicolon on line  38.

> LayoutTests/fast/dom/registerProtocolHandler.html:41
> +// Test that the API correctly parses the url for '%s'

useless comment. also, doesn't actually match the urls it's checking.

> LayoutTests/fast/dom/registerProtocolHandler.html:50
> +        if('SYNTAX_ERR' == e.name) {
> +            succeeded = true;
> +        };

ditto: above

> LayoutTests/fast/dom/registerProtocolHandler.html:57
> +    if(succeeded == true) {
> +        debug('Pass: Invalid url "' + url + '" threw SYNTAX_ERR exception.');
> +    } else {
> +        debug('Fail: Invalid url "' + url + '" allowed.');
> +    };

ditto above

> LayoutTests/fast/dom/registerProtocolHandler.html:72
> +if(succeeded == true) {
> +  debug('Pass: Valid call succeeded.');
> +} else {
> +  debug('Fail: Invalid call did not succeed.');
> +};

ditto re: braces

> LayoutTests/platform/chromium/test_expectations.txt:3044
> +BUGCR11359 : fast/dom/registerProtocolHandler.html = TEXT

Standard webkit practice is to just check in the failing result rather than to skip it on every platform.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.