[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 #10 from James Kozianski <koz at chromium.org>  2011-01-23 16:34:00 PST ---
(In reply to comment #8)
> (In reply to comment #7)
> > (In reply to comment #3)
> > > (From update of attachment 79241 [details] [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] [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.

Cool. Done.

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

Yeah, good point.

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