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

[bugzilla-daemon at webkit.org]

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


David Levin <levin at chromium.org> changed:

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




--- Comment #8 from David Levin <levin at chromium.org>  2011-01-20 14:31:57 PST ---
(In reply to comment #7)
> (In reply to comment #3)
> > (From update of attachment 79241 [details] [details])
> > > Source/WebKit/mac/Configurations/FeatureDefines.xcconfig:130
> > It looks like something went wrong here. This diff doesn't look right.
> 
> Is there anything specific wrong with it you can point out? To my eye it looks similar to other diffs for this line (such as those in bug 49272).

It looks fine in the latest patch.


(In reply to comment #4)
> (From update of attachment 79241 [details])
> 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?)

Not being testable is a valid reason but this is testable. It is exposed to js code.

I would add tests. You can add a few to test error conditions.  You can add the test to Skipped files for platforms that don't have it enabled.

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

I'd avoid that actually. Since this is behind a flag, if a platform enables that flag, then the compile error will let them know exactly what they need to implement to make it work.

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