Skip to content

Add more legacy event types for createEvent() #220

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from

Conversation

ayg
Copy link
Contributor

@ayg ayg commented Apr 14, 2016

Approximately as requested at
https://bugzilla.mozilla.org/show_bug.cgi?id=1251198#c7. This is the
list of events supported in createEvent() by at least two of Firefox,
Chrome, and IE 11. The one exception is I omitted MutationEvent, which
all three support, because apparently spec-land has decided to deny its
existence in the hope that it will go away.

XXX Bikeshed does not know about all of the added interface names and I
don't know how to tell it. (The error is non-fatal, they just aren't
linked.)

Approximately as requested at
<https://bugzilla.mozilla.org/show_bug.cgi?id=1251198#c7>.  This is the
list of events supported in createEvent() by at least two of Firefox,
Chrome, and IE 11.  The one exception is I omitted MutationEvent, which
all three support, because apparently spec-land has decided to deny its
existence in the hope that it will go away.

XXX Bikeshed does not know about all of the added interface names and I
don't know how to tell it.  (The error is non-fatal, they just aren't
linked.)
<tr><td>"<code>htmlevents</code>"
<tr><td>"<code>focusevent</code>"<td>{{FocusEvent}}<td>[[!UIEVENTS]]
<tr><td>"<code>hashchangeevent</code>"<td>{{HashChangeEvent}}<td>[[!HTML]]
<tr><td>"<code>htmlevents</code>"<td>{{Event}}<td>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be clearer to keep this grouped with "event" and "events" and ignore it for sorting purposes. Same for "svgevent" below, didn't know that was a thing. Sad times.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I considered that, but decided it would be confusing if the reader were trying to check if an event is on the list, and that that outweighs the benefit.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Though, maybe note that in the source somehow, so that folks don't try to patch it up.

@ArkadiuszMichalski
Copy link
Contributor

Chrome supports some of them when we use CamelCase notation.

DragEvent: Firefox, Chrome, IE11, Presto
PopStateEvent: Firefox, Chrome, IE11, Presto
StorageEvent: Firefox, Chrome, IE11, Presto

CompositionEvent: Firefox, Chrome, IE11
TextEvent: Firefox (return object CompositionEvent), Chrome and IE11 (return object TextEvent)

BeforeUnloadEvent: Firefox, Chrome
DeviceMotionEvent: Firefox, Chrome:

@ayg
Copy link
Contributor Author

ayg commented Apr 14, 2016

I didn't notice that Chrome and IE return TextEvent -- thanks! But there's no such thing as TextEvent right now in standards-land, and it doesn't seem worth it to define it just for this. So we don't otherwise want to define it, Chrome and IE should get rid of it and return CompositionEvent like Firefox.

Is there anything else in your comment that differs from the commit? It looks like all the events you mentioned are already in the changeset.

@ArkadiuszMichalski
Copy link
Contributor

No, all ok, comments deceived me and I don't looked for a new list.

@annevk
Copy link
Member

annevk commented Apr 15, 2016

Landed as 9e3ce67. @ayg you should have write access to the repository so you can create branches directly on this repository for PRs. That will make merging them a little cleaner.

@annevk annevk closed this Apr 15, 2016
@annevk
Copy link
Member

annevk commented Apr 15, 2016

Also, thank you for cleaning this up!

@ayg
Copy link
Contributor Author

ayg commented Apr 17, 2016

@annevk What makes it cleaner if I create branches directly on the repository?

@ayg ayg deleted the createEvent branch April 17, 2016 12:48
@ayg
Copy link
Contributor Author

ayg commented Apr 17, 2016

(Also, I don't think I have write permissions to the whatwg repositories. It says I don't have the rights to merge pull requests, and once when I tried pushing by accident to the whatwg repo instead of my own, I got a permissions error.)

@annevk
Copy link
Member

annevk commented Apr 18, 2016

@ayg it makes it possible for me to push fixups to the branch and then merge the PR in a way that GitHub understands it as merged and therefore it will link back to it from the commit that ends up on master, giving the repository a bit more history without having to manually add a PR link to the commit.

Push access to master is restricted, but you should have write access for this repository meaning you can create branches and such. Let me know if you need access to other repositories.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants