Bug 65370

Summary: Remove LegacyDefaultOptionalArguments flag from navigator IDL files
Product: WebKit Reporter: Mark Pilgrim (Google) <pilgrim>
Component: New BugsAssignee: Nobody <webkit-unassigned>
Severity: Normal CC: abarth, dglazkov, ossy, rniwa, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Description Flags
Patch none

Description Mark Pilgrim (Google) 2011-07-29 06:08:53 PDT
As discussed in IRC, we are migrating our IDL files away from the interface-level "LegacyDefaultOptionalArguments" flag and onto argument-level [Optional] or [Optional=CallWithDefaultValue] flags. This patch migrates navigator-related files. This patch changes behavior -- it makes the registerProtocolHandler match Firefox's behavior by making all arguments required and throwing TypeError if required arguments are missing. It also adjusts an existing test to check for the TypeError.
Comment 1 Mark Pilgrim (Google) 2011-07-29 06:09:55 PDT
Created attachment 102354 [details]
Comment 2 WebKit Review Bot 2011-07-29 07:03:47 PDT
Comment on attachment 102354 [details]

Attachment 102354 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9266501

New failing tests:
Comment 3 Adam Barth 2011-07-29 10:52:23 PDT
Comment on attachment 102354 [details]

Can you check how registerProtocolHandler works in other browsers?  I'd expect folks might leave off the "title" attribute.
Comment 4 Adam Barth 2011-07-29 10:52:53 PDT
Comment on attachment 102354 [details]

Looks like you're failing a test.
Comment 5 Mark Pilgrim (Google) 2011-08-03 13:27:33 PDT
Firefox throws NOT_ENOUGH_ARGUMENTS exception if any argument is missing, including title.
Comment 6 Adam Barth 2011-08-03 13:44:47 PDT
(In reply to comment #5)
> Firefox throws NOT_ENOUGH_ARGUMENTS exception if any argument is missing, including title.

Great.  Thanks for checking.  Maybe add a test to that effect?
Comment 7 Mark Pilgrim (Google) 2011-08-03 13:49:49 PDT
Created attachment 102821 [details]
Comment 8 WebKit Review Bot 2011-08-03 14:58:57 PDT
Comment on attachment 102821 [details]

Clearing flags on attachment: 102821

Committed r92315: <http://trac.webkit.org/changeset/92315>
Comment 9 WebKit Review Bot 2011-08-03 14:59:02 PDT
All reviewed patches have been landed.  Closing bug.
Comment 10 Ryosuke Niwa 2011-08-03 18:10:00 PDT
fast/dom/navigator-detached-no-crash.html started failing on Qt after this patch:
Comment 11 Adam Barth 2011-08-03 18:13:20 PDT
That makes very little sense.  Maybe I'm confused.
Comment 13 Ryosuke Niwa 2011-08-03 18:38:05 PDT
I don't think registerProtocolHandler is enabled on any port but chromium.  I'll just revert http://trac.webkit.org/browser/trunk/LayoutTests/fast/dom/navigator-detached-no-crash-expected.txt?rev=92315
Comment 14 Ryosuke Niwa 2011-08-03 18:42:04 PDT
Done in http://trac.webkit.org/changeset/92338.