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

Consider removing special cancellation behaviour for event handlers #423

Closed
jdm opened this Issue Dec 18, 2015 · 6 comments

Comments

4 participants
@jdm

jdm commented Dec 18, 2015

While implementing the special cases defined at https://html.spec.whatwg.org/multipage/webappapis.html#the-event-handler-processing-algorithm in Servo, we did some investigations. This test shows interesting results across browsers:

  • Firefox: true, false, true (ie. supports mouseover special case)
  • Safari/Blink: false, false, true (ie. supports no special cases)

I don't have access to IE/Edge to see their results. If nobody implements the special behaviour for onbeforeunload, we should remove it from the spec.

@domenic

This comment has been minimized.

Show comment
Hide comment
@domenic

domenic Dec 18, 2015

Member

Edge gives false, false, true. A PR converging on the Edge/WebKit/Blink behavior sounds great to me!

Member

domenic commented Dec 18, 2015

Edge gives false, false, true. A PR converging on the Edge/WebKit/Blink behavior sounds great to me!

@jdm

This comment has been minimized.

Show comment
Hide comment
@jdm

jdm Dec 18, 2015

http://software.hixie.ch/utilities/js/live-dom-viewer/?saved=3795 adds a check for the onerror handler too; Firefox, Safari and Blink all yield true for that case.

jdm commented Dec 18, 2015

http://software.hixie.ch/utilities/js/live-dom-viewer/?saved=3795 adds a check for the onerror handler too; Firefox, Safari and Blink all yield true for that case.

@jdm

This comment has been minimized.

Show comment
Hide comment
@jdm

jdm Dec 18, 2015

That's a lie, Safari yields false. Nice.

jdm commented Dec 18, 2015

That's a lie, Safari yields false. Nice.

@domenic

This comment has been minimized.

Show comment
Hide comment
@domenic

domenic Dec 18, 2015

Member

false, false, true, false in Edge. Fascinating.

Member

domenic commented Dec 18, 2015

false, false, true, false in Edge. Fascinating.

@annevk

This comment has been minimized.

Show comment
Hide comment
@annevk

annevk Dec 19, 2015

Member

Could it be that browsers vary on synthetic vs non-synthetic event? Would be good to also test the scenario where the user agent dispatches the event before changing things.

Member

annevk commented Dec 19, 2015

Could it be that browsers vary on synthetic vs non-synthetic event? Would be good to also test the scenario where the user agent dispatches the event before changing things.

bors-servo added a commit to servo/servo that referenced this issue Feb 25, 2016

Auto merge of #9755 - jdm:handlerreturn, r=jdm
use return value of invoking event handlers to cancel the event

Rebased from #8707. Fixes #8490. We can modify the code and test as necessary whenever we make a decision about whatwg/html#423 in the future.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.svg" height="40" alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9755)
<!-- Reviewable:end -->

bors-servo added a commit to servo/servo that referenced this issue Feb 26, 2016

Auto merge of #9755 - jdm:handlerreturn, r=jdm
use return value of invoking event handlers to cancel the event

Rebased from #8707. Fixes #8490. We can modify the code and test as necessary whenever we make a decision about whatwg/html#423 in the future.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.svg" height="40" alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9755)
<!-- Reviewable:end -->

bors-servo added a commit to servo/servo that referenced this issue Feb 27, 2016

Auto merge of #9755 - jdm:handlerreturn, r=jdm
use return value of invoking event handlers to cancel the event

Rebased from #8707. Fixes #8490. We can modify the code and test as necessary whenever we make a decision about whatwg/html#423 in the future.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.svg" height="40" alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9755)
<!-- Reviewable:end -->
@bzbarsky

This comment has been minimized.

Show comment
Hide comment
@bzbarsky

bzbarsky Jan 25, 2017

Collaborator

Could it be that browsers vary on synthetic vs non-synthetic event?

Maybe. They definitely vary on Event vs ErrorEvent and various other things. See detailed testcase in #2296.

Note that the testcase linked above at http://software.hixie.ch/utilities/js/live-dom-viewer/?saved=3795 uses Event, not ErrorEvent for onerror, but the spec says to only do the special handling for ErrorEvent to start with. So "4: false" would be correct per spec. The fixed version at http://software.hixie.ch/utilities/js/live-dom-viewer/?saved=4835 shows "4: true" in all browsers, and I'm pretty sure web compat depends on that part.

Collaborator

bzbarsky commented Jan 25, 2017

Could it be that browsers vary on synthetic vs non-synthetic event?

Maybe. They definitely vary on Event vs ErrorEvent and various other things. See detailed testcase in #2296.

Note that the testcase linked above at http://software.hixie.ch/utilities/js/live-dom-viewer/?saved=3795 uses Event, not ErrorEvent for onerror, but the spec says to only do the special handling for ErrorEvent to start with. So "4: false" would be correct per spec. The fixed version at http://software.hixie.ch/utilities/js/live-dom-viewer/?saved=4835 shows "4: true" in all browsers, and I'm pretty sure web compat depends on that part.

domenic added a commit that referenced this issue Feb 24, 2017

domenic added a commit that referenced this issue Feb 24, 2017

Remove special cancelation behavior for mouseover
This was only ever implemented by Gecko. Fixes part of #423.

domenic added a commit that referenced this issue Feb 24, 2017

Fix event handler processing algorithm to match implementations
* Remove the special cancelation behavior for onmouseover. This was only
  ever implemented by Gecko. Fixes #423 in part.
* Update the special cancelation and argument behavior for onerror to a
  reasonable set of intersection semantics among current
  implementations. This fixes #2296, and fixes the rest of #423. See
  especially
  #2296 (comment).
* Editorial: links directly to Web IDL's definition of "invoke" instead
  of indirecting.
* Editorial: clarifies that BeforeUnloadEvents always have type
  "beforeunload", instead of making that part of the conditional.

@domenic domenic self-assigned this Feb 24, 2017

domenic added a commit that referenced this issue Mar 9, 2017

Fix event handler processing algorithm to match implementations
* Remove the special cancelation behavior for onmouseover. This was only
  ever implemented by Gecko. Fixes #423 in part.
* Update the special cancelation and argument behavior for onerror to a
  reasonable set of intersection semantics among current
  implementations. This fixes #2296, and fixes the rest of #423. See
  especially
  #2296 (comment).
* Editorial: links directly to Web IDL's definition of "invoke" instead
  of indirecting.
* Editorial: clarifies that BeforeUnloadEvents always have type
  "beforeunload", instead of making that part of the conditional.

@domenic domenic closed this in #2398 Mar 9, 2017

domenic added a commit that referenced this issue Mar 9, 2017

Fix event handler processing algorithm to match implementations
* Remove the special cancelation behavior for onmouseover. This was only
  ever implemented by Gecko. Fixes #423 in part.
* Update the special cancelation and argument behavior for onerror to a
  reasonable set of intersection semantics among current
  implementations. This fixes #2296, and fixes the rest of #423. See
  especially
  #2296 (comment).
* Editorial: links directly to Web IDL's definition of "invoke" instead
  of indirecting.
* Editorial: clarifies that BeforeUnloadEvents always have type
  "beforeunload", instead of making that part of the conditional.

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Mar 11, 2017

Bug 1345996. Change event handler invocation to only do the "true ret…
…urn cancels" for onerror handlers handed ErrorEvents, and only on globals. r=smaug

See whatwg/html#2296 and
whatwg/html#423 for details on what various browsers
do and whatnot.

MozReview-Commit-ID: DytkZreHudx

xeonchen pushed a commit to mozilla-necko/gecko that referenced this issue Mar 11, 2017

Bug 1345996. Change event handler invocation to only do the "true ret…
…urn cancels" for onerror handlers handed ErrorEvents, and only on globals. r=smaug

See whatwg/html#2296 and
whatwg/html#423 for details on what various browsers
do and whatnot.

MozReview-Commit-ID: DytkZreHudx

Manishearth pushed a commit to Manishearth/gecko-dev that referenced this issue Mar 15, 2017

Bug 1345996. Change event handler invocation to only do the "true ret…
…urn cancels" for onerror handlers handed ErrorEvents, and only on globals. r=smaug

See whatwg/html#2296 and
whatwg/html#423 for details on what various browsers
do and whatnot.

MozReview-Commit-ID: DytkZreHudx

JerryShih pushed a commit to JerryShih/gecko-dev that referenced this issue Mar 16, 2017

Bug 1345996. Change event handler invocation to only do the "true ret…
…urn cancels" for onerror handlers handed ErrorEvents, and only on globals. r=smaug

See whatwg/html#2296 and
whatwg/html#423 for details on what various browsers
do and whatnot.

MozReview-Commit-ID: DytkZreHudx
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment