RESOLVED FIXED 65370
Remove LegacyDefaultOptionalArguments flag from navigator IDL files 
https://bugs.webkit.org/show_bug.cgi?id=65370
Summary Remove LegacyDefaultOptionalArguments flag from navigator IDL files
Mark Pilgrim (Google)
Reported 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.
Attachments
Patch (3.43 KB, patch)
2011-07-29 06:09 PDT, Mark Pilgrim (Google) 		no flags
DetailsFormatted DiffDiff
Patch (4.48 KB, patch)
2011-08-03 13:49 PDT, Mark Pilgrim (Google) 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Mark Pilgrim (Google)
Comment 1 2011-07-29 06:09:55 PDT
Created attachment 102354 [details] Patch
WebKit Review Bot
Comment 2 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
Adam Barth
Comment 3 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.
Adam Barth
Comment 4 2011-07-29 10:52:53 PDT
Comment on attachment 102354 [details] Patch Looks like you're failing a test.
Mark Pilgrim (Google)
Comment 5 2011-08-03 13:27:33 PDT
Firefox throws NOT_ENOUGH_ARGUMENTS exception if any argument is missing, including title.
Adam Barth
Comment 6 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?
Mark Pilgrim (Google)
Comment 7 2011-08-03 13:49:49 PDT
Created attachment 102821 [details] Patch
WebKit Review Bot
Comment 8 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>
WebKit Review Bot
Comment 9 2011-08-03 14:59:02 PDT
All reviewed patches have been landed. Closing bug.
Ryosuke Niwa
Comment 10 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
Adam Barth
Comment 11 2011-08-03 18:13:20 PDT
That makes very little sense. Maybe I'm confused.
Ryosuke Niwa
Comment 13 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
Ryosuke Niwa
Comment 14 2011-08-03 18:42:04 PDT
Done in http://trac.webkit.org/changeset/92338.
Note You need to log in before you can comment on or make changes to this bug.