1675894 - Disconnected text controls does not fire selectionchange on input elements

Status: RESOLVED FIXED

(Core :: DOM: Selection, defect, P3)

Description

[Kagami Rosylight [:saschanaz] (they/them)]

Attachment: selectionchange-input.html

1. Open the attachment
2. See console

Expected: Both lines should have events
Actual: Only the second line has one (actually two, which is bug 1675883)

Comment 1

[Kagami Rosylight [:saschanaz] (they/them)]

input = document.createElement("input");
input.addEventListener("selectionchange", console.log);
input.selectionStart = 4 // does not fire a selectionchange event

In the example in comment #0, the initial selectionStart modification is done before being connected to a frame, and thus ignored.

Comment 2

[Kagami Rosylight [:saschanaz] (they/them)]

Firing selectionchange on disconnected elements would require a different code path, as it currently depends on nsTextControlFrame which is only available when connected to a tree.

Maybe it could make sense to not fire an event for comment #1 case, but I think it should fire one for comment #0 case. Setters probably can flush layout to get a frame in that case (only when NODE_NEEDS_FRAME is set, to prevent performance degradation?).

What do you think, Masayuki?

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to Kagami :saschanaz from comment #2)

Firing selectionchange on disconnected elements would require a different code path, as it currently depends on nsTextControlFrame which is only available when connected to a tree.

Maybe it could make sense to not fire an event for comment #1 case, but I think it should fire one for comment #0 case. Setters probably can flush layout to get a frame in that case (only when NODE_NEEDS_FRAME is set, to prevent performance degradation?).

What do you think, Masayuki?

Well, I wonder, how about the case when <input style="display: none">? If the dispatcher is in the frame, I guess that the event won't be fired in this case, but DOM events shouldn't depend on the layout information...

I have no idea about whether we should or should not fire DOM events on the node which is not in any documents. But I guess we should do it. However, I assume that you're talking about the case of Document node. If so, I think that it shouldn't be fired on the Document node if the event target is disconnected.

So, I think that selectionchange should be fired even in the comment 1's case on the <input>, but shouldn't be fired on the document. But I don't understand the comment 0's case because I see first line has an event for Document and 2 events for <input>. And nothing is appended in the second line. It's different from the "actual result" mentioned in comment 0. I see similar behavior on Blink (about the difference between 1st line and 2nd line in the console). So, I'm not sure what you want to fix here.

Comment 4

[Kagami Rosylight [:saschanaz] (they/them)]

(In reply to Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(Got a cold, working slower) from comment #3)

Well, I wonder, how about the case when <input style="display: none">? If the dispatcher is in the frame, I guess that the event won't be fired in this case, but DOM events shouldn't depend on the layout information...

Makes sense. In that case maybe there should be a way to fire selectionchange just as it currently fires select event without a frame. Not sure it would be useful though, but consistency is important, yes.

I have no idea about whether we should or should not fire DOM events on the node which is not in any documents. But I guess we should do it. However, I assume that you're talking about the case of Document node. If so, I think that it shouldn't be fired on the Document node if the event target is disconnected.

Oops, I meant the case of each input element.

So, I think that selectionchange should be fired even in the comment 1's case on the <input>, but shouldn't be fired on the document. But I don't understand the comment 0's case because I see first line has an event for Document and 2 events for <input>. And nothing is appended in the second line. It's different from the "actual result" mentioned in comment 0. I see similar behavior on Blink (about the difference between 1st line and 2nd line in the console). So, I'm not sure what you want to fix here.

Hmm, interesting. If I hit the refresh button when the focus is in the URL bar, I see no events in the first line and one event for <input>. Not sure why the second line is empty in your machine.

Comment 5

[Kagami Rosylight [:saschanaz] (they/them)]

Attachment: Bug 1675894 - Fire selectionchange from disconnected text controls

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/18c139fe8859
Fire selectionchange from disconnected text controls r=masayuki

Comment 7

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/26768 for changes under testing/web-platform/tests

Comment 8

[Bogdan Tara[:bogdan_tara | bogdant]]

Backed out changeset 18c139fe8859 (bug 1675894) for selectionchange.tentative.html failures.

Push with failure: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&fromchange=b9fa2c214287f5c589d0f85c6b9c100db6fbee2b&searchStr=web%2Cplatform%2Ctests&selectedTaskRun=CxH_tJvnRv2DOTDCxJ9sMA.0&tochange=ac2190ca699b3148efd67a0e9ce724dc42719646

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

Failure log: https://treeherder.mozilla.org/logviewer?job_id=323658294&repo=autoland&lineNumber=9907

[task 2020-12-05T12:37:25.016Z] 12:37:25     INFO - TEST-START | /xhr/xmlhttprequest-timeout-worker-overridesexpires.html?timeout set to non-expiring value after timeout fires
[task 2020-12-05T12:37:25.017Z] 12:37:25     INFO - Closing window 759
[task 2020-12-05T12:37:25.188Z] 12:37:25     INFO - PID 8556 | JavaScript error: , line 0: NotFoundError: No such JSWindowActor 'MarionetteEvents'
[task 2020-12-05T12:37:25.401Z] 12:37:25     INFO - 
[task 2020-12-05T12:37:25.401Z] 12:37:25     INFO - TEST-PASS | /selection/textcontrols/selectionchange.tentative.html | Modifying selectionStart value of the disconnected input element 
[task 2020-12-05T12:37:25.401Z] 12:37:25     INFO - TEST-PASS | /selection/textcontrols/selectionchange.tentative.html | Modifying selectionEnd value of the disconnected input element 
[task 2020-12-05T12:37:25.401Z] 12:37:25     INFO - TEST-PASS | /selection/textcontrols/selectionchange.tentative.html | Calling setSelectionRange() on the disconnected input element 
[task 2020-12-05T12:37:25.401Z] 12:37:25     INFO - TEST-PASS | /selection/textcontrols/selectionchange.tentative.html | Calling select() on the disconnected input element 
[task 2020-12-05T12:37:25.401Z] 12:37:25     INFO - TEST-PASS | /selection/textcontrols/selectionchange.tentative.html | Setting initial zero selectionStart value on the disconnected input element 
[task 2020-12-05T12:37:25.401Z] 12:37:25     INFO - TEST-PASS | /selection/textcontrols/selectionchange.tentative.html | Setting the same selectionStart value twice on the disconnected input element 
[task 2020-12-05T12:37:25.401Z] 12:37:25     INFO - TEST-PASS | /selection/textcontrols/selectionchange.tentative.html | Setting initial zero selectionEnd value on the disconnected input element 
[task 2020-12-05T12:37:25.401Z] 12:37:25     INFO - TEST-PASS | /selection/textcontrols/selectionchange.tentative.html | Setting the same selectionEnd value twice on the disconnected input element 
[task 2020-12-05T12:37:25.402Z] 12:37:25     INFO - TEST-PASS | /selection/textcontrols/selectionchange.tentative.html | Setting initial zero selection range on the disconnected input element 
[task 2020-12-05T12:37:25.402Z] 12:37:25     INFO - TEST-PASS | /selection/textcontrols/selectionchange.tentative.html | Setting the same selection range twice on the disconnected input element 
[task 2020-12-05T12:37:25.402Z] 12:37:25     INFO - TEST-UNEXPECTED-FAIL | /selection/textcontrols/selectionchange.tentative.html | Calling select() twice on the disconnected input element - assert_equals: expected 1 but got 2
[task 2020-12-05T12:37:25.402Z] 12:37:25     INFO - @http://web-platform.test:8000/selection/textcontrols/selectionchange.tentative.html:167:20
[task 2020-12-05T12:37:25.406Z] 12:37:25     INFO - 
[task 2020-12-05T12:37:25.406Z] 12:37:25     INFO - TEST-PASS | /selection/textcontrols/selectionchange.tentative.html | Setting initial zero selectionEnd value on the textarea element 
[task 2020-12-05T12:37:25.407Z] 12:37:25     INFO - TEST-PASS | /selection/textcontrols/selectionchange.tentative.html | Setting the same selectionEnd value twice on the textarea element 
[task 2020-12-05T12:37:25.407Z] 12:37:25     INFO - TEST-PASS | /selection/textcontrols/selectionchange.tentative.html | Setting initial zero selection range on the textarea element 
[task 2020-12-05T12:37:25.407Z] 12:37:25     INFO - TEST-PASS | /selection/textcontrols/selectionchange.tentative.html | Setting the same selection range twice on the textarea element 
[task 2020-12-05T12:37:25.407Z] 12:37:25     INFO - TEST-PASS | /selection/textcontrols/selectionchange.tentative.html | Calling select() twice on the textarea element 
[task 2020-12-05T12:37:25.407Z] 12:37:25     INFO - TEST-PASS | /selection/textcontrols/selectionchange.tentative.html | Modifying selectionStart value of the disconnected textarea element 
[task 2020-12-05T12:37:25.407Z] 12:37:25     INFO - TEST-PASS | /selection/textcontrols/selectionchange.tentative.html | Modifying selectionEnd value of the disconnected textarea element 
[task 2020-12-05T12:37:25.407Z] 12:37:25     INFO - TEST-PASS | /selection/textcontrols/selectionchange.tentative.html | Calling setSelectionRange() on the disconnected textarea element 
[task 2020-12-05T12:37:25.408Z] 12:37:25     INFO - TEST-PASS | /selection/textcontrols/selectionchange.tentative.html | Calling select() on the disconnected textarea element 
[task 2020-12-05T12:37:25.408Z] 12:37:25     INFO - TEST-PASS | /selection/textcontrols/selectionchange.tentative.html | Setting initial zero selectionStart value on the disconnected textarea element 
[task 2020-12-05T12:37:25.408Z] 12:37:25     INFO - TEST-UNEXPECTED-FAIL | /selection/textcontrols/selectionchange.tentative.html | Setting the same selectionStart value twice on the disconnected textarea element - assert_equals: expected 1 but got 2
[task 2020-12-05T12:37:25.408Z] 12:37:25     INFO - @http://web-platform.test:8000/selection/textcontrols/selectionchange.tentative.html:119:20
[task 2020-12-05T12:37:25.408Z] 12:37:25     INFO - 
[task 2020-12-05T12:37:25.408Z] 12:37:25     INFO - TEST-PASS | /selection/textcontrols/selectionchange.tentative.html | Setting initial zero selectionEnd value on the disconnected textarea element 
[task 2020-12-05T12:37:25.408Z] 12:37:25     INFO - TEST-UNEXPECTED-FAIL | /selection/textcontrols/selectionchange.tentative.html | Setting the same selectionEnd value twice on the disconnected textarea element - assert_equals: expected 1 but got 2
[task 2020-12-05T12:37:25.408Z] 12:37:25     INFO - @http://web-platform.test:8000/selection/textcontrols/selectionchange.tentative.html:138:20
[task 2020-12-05T12:37:25.409Z] 12:37:25     INFO - 
[task 2020-12-05T12:37:25.409Z] 12:37:25     INFO - TEST-PASS | /selection/textcontrols/selectionchange.tentative.html | Setting initial zero selection range on the disconnected textarea element 
[task 2020-12-05T12:37:25.409Z] 12:37:25     INFO - TEST-UNEXPECTED-FAIL | /selection/textcontrols/selectionchange.tentative.html | Setting the same selection range twice on the disconnected textarea element - assert_equals: expected 1 but got 2
[task 2020-12-05T12:37:25.409Z] 12:37:25     INFO - @http://web-platform.test:8000/selection/textcontrols/selectionchange.tentative.html:157:20
[task 2020-12-05T12:37:25.409Z] 12:37:25     INFO - 
[task 2020-12-05T12:37:25.409Z] 12:37:25     INFO - TEST-UNEXPECTED-FAIL | /selection/textcontrols/selectionchange.tentative.html | Calling select() twice on the disconnected textarea element - assert_equals: expected 1 but got 2
[task 2020-12-05T12:37:25.409Z] 12:37:25     INFO - @http://web-platform.test:8000/selection/textcontrols/selectionchange.tentative.html:167:20
[task 2020-12-05T12:37:25.410Z] 12:37:25     INFO - TEST-OK | /selection/textcontrols/selectionchange.tentative.html | took 1019ms

Comment 9

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR was closed without merging

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/daabc85e3382
Fire selectionchange from disconnected text controls r=masayuki

Comment 11

[Cosmin Sabou [:CosminS]]

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

Comment 12

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot