Bug 65370 - Remove LegacyDefaultOptionalArguments flag from navigator IDL files
Summary: Remove LegacyDefaultOptionalArguments flag from navigator IDL files
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-07-29 06:08 PDT by Mark Pilgrim (Google)
Modified: 2011-08-03 18:42 PDT (History)
5 users (show)

See Also:


Attachments
Patch (3.43 KB, patch)
2011-07-29 06:09 PDT, Mark Pilgrim (Google)
no flags Details | Formatted Diff | Diff
Patch (4.48 KB, patch)
2011-08-03 13:49 PDT, Mark Pilgrim (Google)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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]
Patch
Comment 2 WebKit Review Bot 2011-07-29 07:03:47 PDT
Comment on attachment 102354 [details]
Patch

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

New failing tests:
fast/dom/navigator-detached-no-crash.html
Comment 3 Adam Barth 2011-07-29 10:52:23 PDT
Comment on attachment 102354 [details]
Patch

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

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]
Patch
Comment 8 WebKit Review Bot 2011-08-03 14:58:57 PDT
Comment on attachment 102821 [details]
Patch

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:
http://build.webkit.org/results/Qt%20Linux%20Release/r92315%20(36108)/fast/dom/navigator-detached-no-crash-pretty-diff.html
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.