1679474 - Remove HTMLInputElement::DispatchSelectEvent and corresponding textarea method

Status: RESOLVED FIXED

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

Description

[Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)]

It's used for synchronously dispatching select event, which must not occur per the spec. It's currently used in Select() (bug 1677263) and PostHandleEvent().

Comment 1

[Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)]

This is probably easy, TextControlState::SetSelectionRange should cover both DispatchSelectEvent and SelectAll.

Comment 2

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

Testcase

1. Go to https://jsfiddle.net/d_toybox/4samc0ke/1/
2. Press left mouse button over "a"
3. Drag mouse to "c"
4. Release the button

Actual Result

• "mousedown event fired"
• "microtask queued from mousedown event listener run"
• "focus event fired"
• "microtask queued from focus event listener run"
• "timer set from mousedown event listener run"
• "timer set from focus event listener run"
• "select event fired"
• "click event fired"
• "microtask queued from click event listener run"
• "timer set from click event listener run"

Expected Result (?)

• "mousedown event fired"
• "microtask queued from mousedown event listener run"
• "focus event fired"
• "microtask queued from focus event listener run"
• "timer set from mousedown event listener run"
• "timer set from focus event listener run"
• "click event fired"
• "microtask queued from click event listener run"
• "select event fired"
• "timer set from click event listener run"

This is the result on Chrome.

Comment 3

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

With a simple fix, it cannot fix my testcase in comment 2 because we set focus at post processing of mousedown event, so at this moment, mouseup has not been queued into the event loop in the content process yet. Therefore, focus event dispatched from the post processing of mousedown event causes "Select All" at focus event post processor in HTMLInputElement (where kagami pointed out). Then, select event is queued and flushed before mouseup event is coming.

Kagami, do you have any idea to test the new behavior?

(Ideally, we should check when (mousedown, mouseup or click) Chrome change Selection in <input> and align the behavior though.)

Comment 4

[Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)]

Then, select event is queued and flushed before mouseup event is coming.

Doesn't that only happen with shouldSelectAllOnFocus being true, which becomes true only by keyboard or calling .focus()? Clicking doesn't seem to have that behavior.

My log says select happens after mouseup. This is the result when I click the front of the text, drag to the right, and finish the click (with a mouseup listener added:

    "mousedown event fired"
    "microtask queued from mousedown event listener run"
    "focus event fired"
    "microtask queued from focus event listener run"
    "timer set from mousedown event listener run"
    "timer set from focus event listener run"
    "mouseup event fired"
    "microtask queued from mouseup event listener run"
    "select event fired"
    "click event fired"
    "microtask queued from click event listener run"
    "timer set from mouseup event listener run"
    "timer set from click event listener run"

Kagami, do you have any idea to test the new behavior?

Synthesize the click, get an array and push the event type on each event, wait a bit to flush all the events, and finally check the event order?

Comment 5

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

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

Then, select event is queued and flushed before mouseup event is coming.

Doesn't that only happen with shouldSelectAllOnFocus being true, which becomes true only by keyboard or calling .focus()? Clicking doesn't seem to have that behavior.

Oh? I'm confused...

My log says select happens after mouseup. This is the result when I click the front of the text, drag to the right, and finish the click (with a mouseup listener added:

    "mousedown event fired"
    "microtask queued from mousedown event listener run"
    "focus event fired"
    "microtask queued from focus event listener run"
    "timer set from mousedown event listener run"
    "timer set from focus event listener run"
    "mouseup event fired"
    "microtask queued from mouseup event listener run"
    "select event fired"
    "click event fired"
    "microtask queued from click event listener run"
    "timer set from mouseup event listener run"
    "timer set from click event listener run"

Thanks, but oddly, <input> does not select the value with clicking the <input> element in Nightly. I'll check it deeper today if I have much time to work on this.

Kagami, do you have any idea to test the new behavior?

Synthesize the click, get an array and push the event type on each event, wait a bit to flush all the events, and finally check the event order?

My point is, how to put an event into the queue between focus and select caused by the call of SelectAll. It seems that toggle event of <details> element may be available, but I'm still confused at the results.

Comment 6

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

Ah, sorry, my test case commented in 2 is completely wrong for this bug. It does not cause a call of HTMLInputElement::DispatchSelectEvent in HTMLInputElement::PostHandleEvent.

Therefore, I wrote new test case which can be tested both in Chrome and Firefox:
https://jsfiddle.net/d_toybox/tre1b4hm/2/

Then, I got the following events in Firefox:

• focus
• select
• toggle
• select

But I got the following events in Chrome:

• focus
• select
• toggle

Then, I tested the patch of Jan, commenting out the DispatchSelectEvent call. Then, I got:

• focus
• toggle
• select

So the event order is still different, but according to the spec of <details>, it should be dispatched synchronously.

And I check same thing with error event of <img>. However, in this case, error event and select event order is also opposite between Firefox and Chrome.

My conclusion is, we cannot check exact same behavior between browsers. Perhaps, we can just check whether there is or no redundant select event and whether setTimeout(, 0) runs before/after select event. (Chrome selects all before dispatching focus event, so we cannot check it in WPT.) Perhaps, the patch should come with a mochitest-plain or a Mozilla specific WPT.

Comment 7

[Edgar Chen [:edgar]]

(In reply to Masayuki Nakano [:masayuki] (he/him)(JST, +0900) from comment #6)

And I check same thing with error event of <img>. However, in this case, error event and select event order is also opposite between Firefox and Chrome.

<img> with src attribute synchronously loads image currently in Gecko and error event is fired asynchronously, so we get the error event before select event is kind of expected.

(Chrome selects all before dispatching focus event, so we cannot check it in WPT.)

So it looks like Chrome schedule select event dispatching before running focus event handler, too.

Comment 8

[Jan Jaeschke [:jjaschke]]

Attachment: Bug 1679474 - Removed DispatchSelectEvent from HTMLInputElement::PostHandleEvent. r=masayuki

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7ab1f393da19
Removed DispatchSelectEvent from HTMLInputElement::PostHandleEvent. r=masayuki

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/630397ba4155
fix lint failure on dispatch_select_event.html. r=fix CLOSED TREE

Comment 11

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/7ab1f393da19
https://hg.mozilla.org/mozilla-central/rev/630397ba4155