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

[bugzilla-daemon at webkit.org]

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





--- Comment #35 from James Kozianski <koz at chromium.org>  2011-01-31 14:52:41 PST ---
(In reply to comment #28)
> 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.

Done.

> 
> > 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;

Done.

> 
> > 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.

Done.

> 
> > 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.

Removed.

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

Done.

> 
> > 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

Done.

> 
> > 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

Done.

> 
> > 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.

Done.

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