Bug 65370

Summary:

Remove LegacyDefaultOptionalArguments flag from navigator IDL files

Product:

WebKit

Reporter:

Mark Pilgrim (Google) <pilgrim>

Component:

New Bugs

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

abarth, dglazkov, ossy, rniwa, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none

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	Details Formatted Diff Diff
Patch (4.48 KB, patch) 2011-08-03 13:49 PDT, Mark Pilgrim (Google)	no flags	Details Formatted Diff Diff
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