Bug 161321 – document.createEvent("popstateevent") should create a PopStateEvent

• Component: DOM
• Status: RESOLVED FIXED

[Chris Dumez]

document.createEvent("popstateevent") should create a PopStateEvent as per:
- https://dom.spec.whatwg.org/#dom-document-createevent

Firefox and Chrome match the specification but WebKit throws an exception, which is risky compatibility-wise.

[Chris Dumez]

Created attachment 287305 [details]
Patch

[Darin Adler]

Comment on attachment 287305 [details]
Patch

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

> Source/WebCore/dom/PopStateEvent.h:58
> +    PopStateEvent() = default;

I don’t think this is right. I think we want to pass some arguments to the Event constructor: Event(eventNames().popstateEvent, false, true).

[Chris Dumez]

Comment on attachment 287305 [details]
Patch

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

>> Source/WebCore/dom/PopStateEvent.h:58
>> +    PopStateEvent() = default;
> 
> I don’t think this is right. I think we want to pass some arguments to the Event constructor: Event(eventNames().popstateEvent, false, true).

Hmm, this is what we do elsewhere. e.g.
    static Ref<Event> createForBindings()
    {
        return adoptRef(*new Event);
    }

    static Ref<CustomEvent> createForBindings()
    {
        return adoptRef(*new CustomEvent);
    }

People are supposed to call init*Event() to initialize the members after using document.createEvent() to construct the event.

Also note step 5 in the spec (link in the changeling):
Let event be the result of invoking the initial value of constructor with the empty string as argument.

[Chris Dumez]

(In reply to comment #3)
> Comment on attachment 287305 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=287305&action=review
> 
> >> Source/WebCore/dom/PopStateEvent.h:58
> >> +    PopStateEvent() = default;
> > 
> > I don’t think this is right. I think we want to pass some arguments to the Event constructor: Event(eventNames().popstateEvent, false, true).
> 
> Hmm, this is what we do elsewhere. e.g.
>     static Ref<Event> createForBindings()
>     {
>         return adoptRef(*new Event);
>     }
> 
>     static Ref<CustomEvent> createForBindings()
>     {
>         return adoptRef(*new CustomEvent);
>     }
> 
> People are supposed to call init*Event() to initialize the members after
> using document.createEvent() to construct the event.
> 
> Also note step 5 in the spec (link in the changeling):
> Let event be the result of invoking the initial value of constructor with
> the empty string as argument.

So as per the spec, this should be equivalent to calling:
new PopStateEvent("");

[Chris Dumez]

(In reply to comment #4)
> (In reply to comment #3)
> > Comment on attachment 287305 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=287305&action=review
> > 
> > >> Source/WebCore/dom/PopStateEvent.h:58
> > >> +    PopStateEvent() = default;
> > > 
> > > I don’t think this is right. I think we want to pass some arguments to the Event constructor: Event(eventNames().popstateEvent, false, true).
> > 
> > Hmm, this is what we do elsewhere. e.g.
> >     static Ref<Event> createForBindings()
> >     {
> >         return adoptRef(*new Event);
> >     }
> > 
> >     static Ref<CustomEvent> createForBindings()
> >     {
> >         return adoptRef(*new CustomEvent);
> >     }
> > 
> > People are supposed to call init*Event() to initialize the members after
> > using document.createEvent() to construct the event.
> > 
> > Also note step 5 in the spec (link in the changeling):
> > Let event be the result of invoking the initial value of constructor with
> > the empty string as argument.
> 
> So as per the spec, this should be equivalent to calling:
> new PopStateEvent("");

So I do believe this patch is correct as per the specification. The values of event.type as well as the Event flags are also converted by dom/nodes/Document-createEvent.html which we PASS for PopStateEvent after my patch.

From Test:
function testAlias(arg, iface) {
  var ev;
  test(function() {
    ev = document.createEvent(arg);
    assert_true(ev instanceof window[iface]);
    assert_true(ev instanceof Event);
  }, arg + " should be an alias for " + iface + ".");
  test(function() {
    assert_equals(ev.type, "",
                  "type should be initialized to the empty string");
    assert_equals(ev.target, null,
                  "target should be initialized to null");
    assert_equals(ev.currentTarget, null,
                  "currentTarget should be initialized to null");
    assert_equals(ev.eventPhase, 0,
                  "eventPhase should be initialized to NONE (0)");
    assert_equals(ev.bubbles, false,
                  "bubbles should be initialized to false");
    assert_equals(ev.cancelable, false,
                  "cancelable should be initialized to false");
    assert_equals(ev.defaultPrevented, false,
                  "defaultPrevented should be initialized to false");
    assert_equals(ev.isTrusted, false,
                  "isTrusted should be initialized to false");
  }, "createEvent('" + arg + "') should be initialized correctly.");
}

[WebKit Commit Bot]

Comment on attachment 287305 [details]
Patch

Clearing flags on attachment: 287305

Committed r205138: <http://trac.webkit.org/changeset/205138>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.