Bug 227493 – Support <dialog> element close event

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Tim Nguyen (:ntim)]

The <dialog> element should emit a "close" event.

https://developer.mozilla.org/en-US/docs/Web/API/HTMLDialogElement/close_event

https://html.spec.whatwg.org/multipage/indices.html#event-close

[Radar WebKit Bug Importer]

<rdar://problem/79952910>

[Tim Nguyen (:ntim)]

Created attachment 432575 [details]
Patch

[Antti Koivisto]

Comment on attachment 432575 [details]
Patch

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

> Source/WebCore/html/HTMLDialogElement.h:28
> +#include "EventSender.h"

You can forward declare EventSender with

template<typename T> class EventSender;

so you don't need this include in the header (slightly reducing header bloat and so improving compile times). See the other EventSenders.

> Source/WebCore/html/HTMLDialogElement.h:33
> +typedef EventSender<HTMLDialogElement> DialogEventSender;

Modern replacement for typedef is using:

using DialogEventSender = EventSender<HTMLDialogElement>;

I find it more readable.

> Source/WebCore/html/HTMLDialogElement.h:57
> -    void toggleOpen();
> +    void toggleOpen(bool newValue);

This should be renamed to setOpen since it is now a setter rather than a toggle.
No need for parameter name, it is obvious from context.

[Tim Nguyen (:ntim)]

Created attachment 432578 [details]
Patch

[EWS]

Committed r279406 (239271@main): <https://commits.webkit.org/239271@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 432578 [details].

[Darin Adler]

Comment on attachment 432575 [details]
Patch

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

>> Source/WebCore/html/HTMLDialogElement.h:28
>> +#include "EventSender.h"
> 
> You can forward declare EventSender with
> 
> template<typename T> class EventSender;
> 
> so you don't need this include in the header (slightly reducing header bloat and so improving compile times). See the other EventSenders.

In fact, can do this:

    template<typename> class EventSender;