1452569 - Implement Event's returnValue

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, enhancement, P3)

Description

[Anne (:annevk)]

Since we're adding Event's srcElement and Window's event, we should also add Event's returnValue, which is the last of (formerly) proprietary event extensions.

Standard: https://github.com/whatwg/dom/pull/626.

Note that the standard has made Event's returnValue less powerful than it currently is in Chrome, Edge, and Safari so that it's not a more powerful API than defaultPrevented and preventDefault(). It's hoped to be web compatible, but not yet field tested.

Comment 1

[Chris Rebert]

MDN doesn't yet acknowledge the spec addition or this bug, and so will need updating:
https://developer.mozilla.org/en-US/docs/Web/API/Event/returnValue

Comment 2

[Mike Taylor [:miketaylr]]

(In reply to Chris Rebert from comment #1)
> MDN doesn't yet acknowledge the spec addition or this bug, and so will need
> updating:
> https://developer.mozilla.org/en-US/docs/Web/API/Event/returnValue

You can use the dev-doc-needed keyword for that, thanks!

Comment 3

[Henri Sivonen (:hsivonen)]

Quick implementation notes:
 * This should be easy to implement by using the pre-existing implementations for preventDefault() and defaultPrevented.
 * _canceled_ seems to be called mDefaultPrevented: https://searchfox.org/mozilla-central/source/widget/BasicEvents.h#83
 * The main oddity seems to be that our preventDefault() tracks whether it was called from chrome or content. returnValue should probably be [NeedsCallerType], too, in WebIDL.

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

FWIW, the reason for defaultPrevented checking caller type is that web pages shouldn't see if only browser chrome has called preventDefaul()

Comment 5

[Henri Sivonen (:hsivonen)]

Yeah, but the question is, do we trust chrome code never to set returnValue if we add returnValue? Seems safe to assume that someone might set it from chrome code if we add it.

Comment 6

[Alphan Chen [:alchen]]

Attachment: Bug 1452569 - Implement Event's returnValue. r=smaug

Add retrunValue into Event's interface

Comment 7

[Alphan Chen [:alchen]]

(In reply to Alphan Chen [:alchen] from comment #6)
> Created attachment 9003481 [details]
> Bug 1452569 - Implement Event's returnValue
> 
> Add retrunValue into Event's interface


https://github.com/web-platform-tests/wpt/pull/10258
With this patch, we can pass the tests which related to returnValue.


All tests are PASS.
http://w3c-test.org/dom/events/AddEventListenerOptions-passive.html
http://w3c-test.org/dom/events/Event-constructors.html
http://w3c-test.org/dom/events/Event-defaultPrevented-after-dispatch.html
http://w3c-test.org/dom/events/Event-defaultPrevented.html
http://w3c-test.org/dom/events/Event-initEvent.html
http://w3c-test.org/dom/events/Event-returnValue.html
http://w3c-test.org/dom/events/EventTarget-dispatchEvent-returnvalue.html

FAILED on several tests which are not related to returnValue.
http://w3c-test.org/dom/events/Event-dispatch-click.html

Comment 8

[Alphan Chen [:alchen]]

Comment on attachment 9003481 [details]
Bug 1452569 - Implement Event's returnValue. r=smaug

Hi Oli,

Could you take a look?

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

Sure, could you just put it to my review queue and I'm more likely to see the patch :)

Don't you need to update some .ini files, like https://searchfox.org/mozilla-central/source/testing/web-platform/meta/dom/events/Event-returnValue.html.ini, if the tests are now passing?

Comment 10

[Alphan Chen [:alchen]]

(In reply to Olli Pettay [:smaug] from comment #9)
> Sure, could you just put it to my review queue and I'm more likely to see
> the patch :)
> 
> Don't you need to update some .ini files, like
> https://searchfox.org/mozilla-central/source/testing/web-platform/meta/dom/
> events/Event-returnValue.html.ini, if the tests are now passing?

Thanks. I've revised the patch and request a review.

Comment 11

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 9003481 [details]
Bug 1452569 - Implement Event's returnValue. r=smaug

Olli Pettay [:smaug] has approved the revision.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8427667b7c9a
Implement Event's returnValue. r=smaug

Comment 13

[Cosmin Sabou [:CosminS]]

Backed out for wpt failures on /dom/interfaces.html

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&filter-searchStr=Linux%20x64%20opt%20Web%20platform%20tests%20with%20e10s%20test-linux64%2Fopt-web-platform-tests-e10s-2%20W-e10s(wpt2)&fromchange=8427667b7c9aef4aa8ece1f27254ac9332c6a62c&tochange=315e96b7fc527ecf3b85c8a634663bf6bb19fd00&filter-resultStatus=testfailed&filter-resultStatus=success&filter-resultStatus=runnable&selectedJob=196043310

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=196043310&repo=autoland&lineNumber=2485

Backout link: https://hg.mozilla.org/integration/autoland/rev/315e96b7fc527ecf3b85c8a634663bf6bb19fd00

[task 2018-08-27T08:49:30.559Z] 08:49:30     INFO - TEST-START | /dom/interfaces.html?exclude=Node
[task 2018-08-27T08:49:31.485Z] 08:49:31     INFO - 
[task 2018-08-27T08:49:31.486Z] 08:49:31     INFO - TEST-PASS | /dom/interfaces.html?exclude=Node | Event interface: constant AT_TARGET on interface object 
[task 2018-08-27T08:49:31.486Z] 08:49:31     INFO - TEST-PASS | /dom/interfaces.html?exclude=Node | Event interface: constant AT_TARGET on interface prototype object 
[task 2018-08-27T08:49:31.486Z] 08:49:31     INFO - TEST-PASS | /dom/interfaces.html?exclude=Node | Event interface: constant BUBBLING_PHASE on interface object 
[task 2018-08-27T08:49:31.487Z] 08:49:31     INFO - TEST-PASS | /dom/interfaces.html?exclude=Node | Event interface: constant BUBBLING_PHASE on interface prototype object 
[task 2018-08-27T08:49:31.487Z] 08:49:31     INFO - TEST-PASS | /dom/interfaces.html?exclude=Node | Event interface: attribute eventPhase 
[task 2018-08-27T08:49:31.487Z] 08:49:31     INFO - TEST-PASS | /dom/interfaces.html?exclude=Node | Event interface: operation stopPropagation() 
[task 2018-08-27T08:49:31.487Z] 08:49:31     INFO - TEST-PASS | /dom/interfaces.html?exclude=Node | Event interface: attribute cancelBubble 
[task 2018-08-27T08:49:31.488Z] 08:49:31     INFO - TEST-PASS | /dom/interfaces.html?exclude=Node | Event interface: operation stopImmediatePropagation() 
[task 2018-08-27T08:49:31.488Z] 08:49:31     INFO - TEST-PASS | /dom/interfaces.html?exclude=Node | Event interface: attribute bubbles 
[task 2018-08-27T08:49:31.488Z] 08:49:31     INFO - TEST-PASS | /dom/interfaces.html?exclude=Node | Event interface: attribute cancelable 
[task 2018-08-27T08:49:31.489Z] 08:49:31     INFO - TEST-UNEXPECTED-PASS | /dom/interfaces.html?exclude=Node | Event interface: attribute returnValue - expected FAIL
[task 2018-08-27T08:49:31.489Z] 08:49:31     INFO - TEST-INFO | expected FAIL
[task 2018-08-27T08:49:31.494Z] 08:49:31     INFO - 
[task 2018-08-27T08:49:31.495Z] 08:49:31     INFO - TEST-PASS | /dom/interfaces.html?exclude=Node | Event interface: document.createEvent("Event") must inherit property "NONE" with the proper type 
[task 2018-08-27T08:49:31.496Z] 08:49:31     INFO - TEST-PASS | /dom/interfaces.html?exclude=Node | Event interface: document.createEvent("Event") must inherit property "CAPTURING_PHASE" with the proper type 
[task 2018-08-27T08:49:31.496Z] 08:49:31     INFO - TEST-PASS | /dom/interfaces.html?exclude=Node | Event interface: document.createEvent("Event") must inherit property "AT_TARGET" with the proper type 
[task 2018-08-27T08:49:31.497Z] 08:49:31     INFO - TEST-PASS | /dom/interfaces.html?exclude=Node | Event interface: document.createEvent("Event") must inherit property "BUBBLING_PHASE" with the proper type 
[task 2018-08-27T08:49:31.497Z] 08:49:31     INFO - TEST-PASS | /dom/interfaces.html?exclude=Node | Event interface: document.createEvent("Event") must inherit property "eventPhase" with the proper type 
[task 2018-08-27T08:49:31.498Z] 08:49:31     INFO - TEST-PASS | /dom/interfaces.html?exclude=Node | Event interface: document.createEvent("Event") must inherit property "stopPropagation()" with the proper type 
[task 2018-08-27T08:49:31.498Z] 08:49:31     INFO - TEST-PASS | /dom/interfaces.html?exclude=Node | Event interface: document.createEvent("Event") must inherit property "cancelBubble" with the proper type 
[task 2018-08-27T08:49:31.499Z] 08:49:31     INFO - TEST-PASS | /dom/interfaces.html?exclude=Node | Event interface: document.createEvent("Event") must inherit property "stopImmediatePropagation()" with the proper type 
[task 2018-08-27T08:49:31.499Z] 08:49:31     INFO - TEST-PASS | /dom/interfaces.html?exclude=Node | Event interface: document.createEvent("Event") must inherit property "bubbles" with the proper type 
[task 2018-08-27T08:49:31.500Z] 08:49:31     INFO - TEST-PASS | /dom/interfaces.html?exclude=Node | Event interface: document.createEvent("Event") must inherit property "cancelable" with the proper type 
[task 2018-08-27T08:49:31.500Z] 08:49:31     INFO - TEST-UNEXPECTED-PASS | /dom/interfaces.html?exclude=Node | Event interface: document.createEvent("Event") must inherit property "returnValue" with the proper type - expected FAIL
[task 2018-08-27T08:49:31.500Z] 08:49:31     INFO - TEST-INFO | expected FAIL

Comment 14

[Olli Pettay [:smaug][[e-mail]]]

r+ if you fix those TEST-UNEXPECTED-PASSes by removing relevant lines from .ini

Comment 15

[Alphan Chen [:alchen]]

Attachment: Bug 1452569 - Implement Event's returnValue. r=smaug

Add retrunValue into Event's interface

Comment 16

[Alphan Chen [:alchen]]

Comment on attachment 9004209 [details]
Bug 1452569 - Implement Event's returnValue. r=smaug

Should update the patch in the original page.

Comment 17

[Alphan Chen [:alchen]]

Update patch for fixing TEST-UNEXPECTED-PASSes on /dom/interfaces.html.

Try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ac648c2b3c2a305d083a497dc9474045e9426a67
https://treeherder.mozilla.org/#/jobs?repo=try&revision=dd5beead52566867eacb28e0e2882bf5268ce44c

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/707675409147
Implement Event's returnValue. r=smaug

Comment 19

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/707675409147

Comment 20

[Kohei Yoshino]

Updated the site compatibility note: https://www.fxsitecompat.com/en-CA/docs/2018/support-for-window-event-and-event-returnvalue-has-been-added/

Comment 21

[Eric Shepherd [:sheppy]]

Looking at the spec, there's a comment on the IDL at https://dom.spec.whatwg.org/#interface-event that says "historical" next to returnValue.

My question: what does that mean? Is it deprecated? Or is it just old? :)

I ask because I need to update the docs, and they currently call this property both deprecated and non-standard. It's no longer non-standard, but I need to sort out whether or not to call it deprecated. :)

Comment 22

[Mike Taylor [:miketaylr]]

In the context of the DOM spec, historical more or less means "this is a legacy API that is needed for compat reasons". I think the following comment for HTMLCollection captures the essense pretty well:

> HTMLCollection is a historical artifact we cannot rid the web of. While developers are of course welcome to keep using it, new API standard designers ought not to use it (use sequence<T> in IDL instead). 

Personally, I wouldn't call it deprecated.

Comment 23

[Eric Shepherd [:sheppy]]

Thanks! That's exactly what I thought, but it was worth being sure.

Documentation updated:

https://developer.mozilla.org/en-US/docs/Web/API/Event/returnValue
https://developer.mozilla.org/en-US/docs/Web/API/Event

BCD PR submitted: https://github.com/mdn/browser-compat-data/pull/2756

And listed on Firefox 63 for developers.