[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 #4 from James Kozianski <koz at chromium.org>  2011-01-19 18:37:07 PST ---
(From update of attachment 79241)
View in context: https://bugs.webkit.org/attachment.cgi?id=79241&action=review

>> Source/WebCore/ChangeLog:14
>> +        No new tests. (OOPS!)
> 
> This should either point out the test or explain why none is possible.

Done. (Is something behind behind a compile flag a valid reason for not having a layout test?)

>> Source/WebCore/page/ChromeClient.h:139
>> +        virtual void registerProtocolHandler(const String&, const String&, const String&, const String&) { }
> 
> I think this should be = 0 and be behind the if ENABLE (and have the parameter names filled in).

Done (I added this to all the implementors of ChromeClient I could find, too).

>> Source/WebCore/page/Navigator.cpp:28
>> +#include "ExceptionCode.h"
> 
> Out of order.

Fixed.

>> Source/WebCore/page/Navigator.cpp:178
>> +    // The specification requires that it is a SYNTAX_ERR if the the "%s" token
> 
> "the the"

Fixed.

>>> Source/WebCore/page/Navigator.cpp:190
>>> +    newURL.remove(index, sizeof(token) / sizeof(token[0]));
>> 
>> sizeof(token) - 1, right?
> 
> Also use WTF_ARRAY_LENGTH (if possible). (It may not be possible due to the variable begin a static const local.)

Yep, you're right, Adam.

And WTF_ARRAY_LENGTH does work here, so I've changed it to WTF_ARRAY_LENGTH() - 1.

>> Source/WebCore/page/Navigator.h:66
>> +        void registerProtocolHandler(const String& scheme, const String& url, const String& title, ExceptionCode&);
> 
> This appears to be indented too far.

Fixed.

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