Bug 110131 – Add event dispatch class for the new calendar picker

• Component: Forms
• Status: RESOLVED FIXED

[Keishi Hattori]

We need to add an event dispatch class that will be used in the new calendar picker

[Keishi Hattori]

Created attachment 188904 [details]
Patch

[Build Bot]

Comment on attachment 188904 [details]
Patch

Attachment 188904 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/16637067

[Kent Tamura]

Comment on attachment 188904 [details]
Patch

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

> Source/WebCore/Resources/pagepopups/calendarPicker.js:690
> +    if (!this._callbacks)
> +        return;
> +    var callbacksForType = this._callbacks[type];
> +    if (!callbacksForType)
> +        return;

The function does nothing in a case that 'callback' is not registered.  Should we warn in such case, or should we accept such call sites?

> Source/WebCore/Resources/pagepopups/calendarPicker.js:691
> +    callbacksForType.splice(callbacksForType.indexOf(callback), 1);

What should happen if 'callback' is not a member of 'callbacksForType'?

[Keishi Hattori]

(In reply to comment #3)
> (From update of attachment 188904 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=188904&action=review
> 
> > Source/WebCore/Resources/pagepopups/calendarPicker.js:690
> > +    if (!this._callbacks)
> > +        return;
> > +    var callbacksForType = this._callbacks[type];
> > +    if (!callbacksForType)
> > +        return;
> 
> The function does nothing in a case that 'callback' is not registered.  Should we warn in such case, or should we accept such call sites?
> 
> > Source/WebCore/Resources/pagepopups/calendarPicker.js:691
> > +    callbacksForType.splice(callbacksForType.indexOf(callback), 1);
> 
> What should happen if 'callback' is not a member of 'callbacksForType'?

Like Element.removeEventListener, if the callback is not registered there will be no effect.

I kind of feel that this is better because of its simplicity. We usually don't care if the callback exists or not and we just want it gone. For example when calling CalendarPicker.cleanup() you don't know which callbacks have been set (it might be in the middle of an animation) but you know you want to remove them.

[Kent Tamura]

Comment on attachment 188904 [details]
Patch

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

> Source/WebCore/ChangeLog:8
> +
> +        No new tests. Code is not yet used.

Please mention that this patch is a part of Bug 109439.

[Keishi Hattori]

Created attachment 189477 [details]
Patch

[WebKit Review Bot]

Comment on attachment 189477 [details]
Patch

Clearing flags on attachment: 189477

Committed r143576: <http://trac.webkit.org/changeset/143576>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.