Bug 160085 - Fix default parameter values for window.alert() / prompt() / confirm()
Summary: Fix default parameter values for window.alert() / prompt() / confirm()
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: WebExposed
Depends on:
Blocks:
 
Reported: 2016-07-22 09:26 PDT by Chris Dumez
Modified: 2016-07-22 13:35 PDT (History)
9 users (show)

See Also:


Attachments
Patch (6.56 KB, patch)
2016-07-22 10:21 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (6.68 KB, patch)
2016-07-22 12:13 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2016-07-22 09:26:03 PDT
Fix default parameter values for window.alert() / prompt() / confirm() to match the specification:
- https://html.spec.whatwg.org/multipage/browsers.html#the-window-object

They should default to the empty string, not the string "undefined".

Firefox and chrome agree with the specification.
Comment 1 Chris Dumez 2016-07-22 10:21:50 PDT
Created attachment 284343 [details]
Patch
Comment 2 Darin Adler 2016-07-22 11:30:56 PDT
Comment on attachment 284343 [details]
Patch

Looks like the GTK build failure is real.
Comment 3 Chris Dumez 2016-07-22 12:13:55 PDT
Created attachment 284360 [details]
Patch
Comment 4 Chris Dumez 2016-07-22 12:34:15 PDT
GTK is now building properly.
Comment 5 Ryosuke Niwa 2016-07-22 13:11:13 PDT
Comment on attachment 284360 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=284360&action=review

> Source/WebCore/page/DOMWindow.idl:79
> +#if defined(LANGUAGE_JAVASCRIPT) && LANGUAGE_JAVASCRIPT
> +    void alert();
> +#endif
> +    void alert(DOMString message);

Doesn't this break Objective-C bindings when alert is called without an argument?
Comment 6 Chris Dumez 2016-07-22 13:12:32 PDT
(In reply to comment #5)
> Comment on attachment 284360 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=284360&action=review
> 
> > Source/WebCore/page/DOMWindow.idl:79
> > +#if defined(LANGUAGE_JAVASCRIPT) && LANGUAGE_JAVASCRIPT
> > +    void alert();
> > +#endif
> > +    void alert(DOMString message);
> 
> Doesn't this break Objective-C bindings when alert is called without an
> argument?

We had this discussion on another bug already. optional has no impact on ObjC bindings, so no.
Comment 7 Ryosuke Niwa 2016-07-22 13:14:22 PDT
Comment on attachment 284360 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=284360&action=review

>>> Source/WebCore/page/DOMWindow.idl:79
>>> +    void alert(DOMString message);
>> 
>> Doesn't this break Objective-C bindings when alert is called without an argument?
> 
> We had this discussion on another bug already. optional has no impact on ObjC bindings, so no.

Okay. I didn't see your reply.  I guess someone else reviewed it?
Comment 8 Chris Dumez 2016-07-22 13:15:40 PDT
(In reply to comment #7)
> Comment on attachment 284360 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=284360&action=review
> 
> >>> Source/WebCore/page/DOMWindow.idl:79
> >>> +    void alert(DOMString message);
> >> 
> >> Doesn't this break Objective-C bindings when alert is called without an argument?
> > 
> > We had this discussion on another bug already. optional has no impact on ObjC bindings, so no.
> 
> Okay. I didn't see your reply.  I guess someone else reviewed it?

I am referring to https://bugs.webkit.org/show_bug.cgi?id=160060#c3
Comment 9 WebKit Commit Bot 2016-07-22 13:35:18 PDT
Comment on attachment 284360 [details]
Patch

Clearing flags on attachment: 284360

Committed r203612: <http://trac.webkit.org/changeset/203612>
Comment 10 WebKit Commit Bot 2016-07-22 13:35:24 PDT
All reviewed patches have been landed.  Closing bug.