Remove HTMLInputElement::DispatchSelectEvent and corresponding textarea method
Categories
(Core :: DOM: Selection, defect, P3)
Tracking
()
Tracking | Status | |
---|---|---|
firefox106 | --- | fixed |
People
(Reporter: saschanaz, Assigned: jjaschke)
References
(Blocks 1 open bug, )
Details
(Keywords: good-first-bug)
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
Details |
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()
.
Updated•4 years ago
|
Reporter | ||
Comment 1•2 years ago
|
||
This is probably easy, TextControlState::SetSelectionRange
should cover both DispatchSelectEvent
and SelectAll
.
Updated•2 years ago
|
Comment 2•2 years ago
|
||
Testcase
- Go to https://jsfiddle.net/d_toybox/4samc0ke/1/
- Press left mouse button over "a"
- Drag mouse to "c"
- 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•2 years ago
|
||
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.)
Reporter | ||
Comment 4•2 years ago
|
||
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•2 years ago
|
||
(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 aftermouseup
. 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•2 years ago
|
||
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•2 years ago
|
||
(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 andselect
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.
Assignee | ||
Updated•2 years ago
|
Assignee | ||
Comment 8•2 years ago
|
||
Comment 10•2 years ago
|
||
Comment 11•2 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/7ab1f393da19
https://hg.mozilla.org/mozilla-central/rev/630397ba4155
Description
•