[UI Events][Input Events] Ship "beforeinput" event and `InputEvent.getTargetRanges()` in Nightly (and early beta) channel
Categories
(Core :: DOM: Events, task, P2)
Tracking
()
Tracking | Status | |
---|---|---|
firefox83 | --- | fixed |
People
(Reporter: masayuki, Assigned: masayuki)
References
Details
(Keywords: dev-doc-complete)
Attachments
(6 files)
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review |
Perhaps, we need QA with real web sites before shipping beforeinput
event, and some web apps may require InputEvent.getTargetRanges()
if they want to modify target ranges before browsers do default action.
Assignee | ||
Comment 1•4 years ago
|
||
FYI: You can turn it enabled with dom.input_events.beforeinput.enabled
in about:config
.
Comment 2•4 years ago
•
|
||
For documentation purposes: this was implemented in Firefox 74; awaiting on-by-default to list it as supported in BCD. Be sure to also remove it from the DOM section in "Experimental features in Firefox".
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 3•4 years ago
|
||
Finally, I must have filed necessary tasks for writing patches for this bug. In my current plan, we should make getTargetRanges()
should include invisible white-spaces which should be removed to avoid making them visible accidentally. However, we should not include newly empty ancestor inline/block elements for compatibility with both Blink and WebKit. And for now, I won't collect actual deleting range in table. It requires more refactoring, but not so important for shipping in Nightly channel. And also, I won't care about deletion with multiple selection ranges because it requires a lot of traditional behavior changes (i.e., risky for web-compat) and not supported by the other browsers and users must not use multiple selection ranges in editor.
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 4•4 years ago
|
||
Enabling beforeinput
event in Nightly seems that it does not cause perf regression at least for Raptor.
https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=d3f42b67d723dbe523f0c5c12d74aad923ba5ca6&newProject=try&newRevision=e65f61f0cc0b9f7ab826bb79a259b91f078d91c0&framework=10
Assignee | ||
Comment 5•4 years ago
|
||
Assignee | ||
Comment 6•4 years ago
|
||
The test enables beforeinput
pref at getting focus. However, before that,
each element whose onbeforeinput
is used to check the test result is created
and whose onbeforeinput
is set. In this order, onbeforeinput
isn't
available as an event listener attribute.
This patch makes the test create and initialize <input>
and <textarea>
elements after enabling beforeinput
.
Depends on D91860
Assignee | ||
Comment 7•4 years ago
|
||
Although I don't understand 100% of the reason why it fails to enable
onbeforeinput
attribute with enabling the pref at first add_task()
,
it seems that it's too late to enable it (I guess that JIT compiler refers
GlobalEventHandlers
before enabling it). Anyway, I don't find a way
to test it with dynamically enables beforeinput
events in it.
Therefore, this patch rewrites the test without add_task()
and make it
more stable at least on my environment. Even with this change, I see orange
in my environment, but I guess that SimpleTest.promiseClipboardChange()
has
a bug of something making it unstable.
Depends on D91861
Assignee | ||
Comment 8•4 years ago
|
||
If beforeinput
event is enabled, the assertion count hitting in
WSRunScanner::GetEditableBlockParentOrTopmotEditableInlineContent()
becomes 0.
The reason is, AutoEditActionDataSetter::MaybeDispatchBeforeInputEvent()
gets nullptr
as the result of HTMLEditor.GetInputEventTargetElement()
.
Then, it returns error and every edit action handlers stop handling it.
However, for the main purpose of beforeinput
, i.e., overriding browsers'
default action, we need to dispatch beforeinput
event even in such case.
(I.e., web apps may want to do something around the non-editable node.)
Therefore, this patch makes HTMLEditor.GetInputEventTargetElement()
scan
editing host by itself if HTMLEditor.GetActiveEditingHost()
returns nullptr
due to non-editable focus node. And also make test_bug430392.html
check
whether beforeinput
events and input
events are fired as expected.
I think that I should add new WPT to check nested editing host cases for
considering beforeinput
and input
event target, but I'd like to do it
in another bug for shipping beforeinput
in Nightly channel as soon as
possible.
Depends on D91862
Assignee | ||
Comment 9•4 years ago
|
||
editor/libeditor/tests/test_bug974309.html
and
editor/libeditor/tests/test_bug1268736.html
start to throw exception if
beforeinput
event is enabled. They test XPCOM editor API when a non-editable
element has focus and it's outside of any editing hosts. In this case,
we shouldn't throw an exception for backward compatibility. Although the
all callers should stop handling any edit action in this case, let's make
MaybeDispatchBeforeInputEvent()
return a special success code instead of
an error code for making all callers keep working with traditional paths.
Depends on D91863
Assignee | ||
Comment 10•4 years ago
|
||
Now, InputEvent.getTargetRanges()
returns better ranges when deleting
selected ranges (both collapsed cases and non-collapsed cases, and the
result is similar to Blink). So, beforeinput
events listeners can treat
deletion.
Therefore, we can ship beforeinput
event for getting more feedback from
our testers.
Note that both of them are behind same pref because getTargetRanges()
is required only by beforeinput
event listeners and it may be used
for feature detection of beforeinput
event because Blink does not support
onbeforeinput
event handler attribute.
Depends on D91864
Updated•4 years ago
|
Comment 11•4 years ago
|
||
Pushed by masayuki@d-toybox.com: https://hg.mozilla.org/integration/autoland/rev/b8e7854d72ce Fix a simple bug of `test_dom_keyboard_event.html` r=smaug https://hg.mozilla.org/integration/autoland/rev/ead7db3e08f7 Make `test_input_event.html` put off to initialize elements whose `onbeforeinput` will be used r=smaug https://hg.mozilla.org/integration/autoland/rev/2a6bac028754 Make run all tests in `test_clipboard_events.html` with a child window r=smaug https://hg.mozilla.org/integration/autoland/rev/3ad77c14180a Fix new failure of `test_bug430392.html` when `beforeinput` is enabled r=smaug https://hg.mozilla.org/integration/autoland/rev/e9803b0d9c1d Make `AutoEditActionDataSetter::MaybeDispatchBeforeInputEvent()` stop returning error when there is no proper event target of `beforeinput` event r=smaug https://hg.mozilla.org/integration/autoland/rev/55d1325436c1 Enable `beforeinput` event and `InputEvent.getTargetRanges()` by default in Nightly channel and early Beta r=smaug
Comment 12•4 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/b8e7854d72ce
https://hg.mozilla.org/mozilla-central/rev/ead7db3e08f7
https://hg.mozilla.org/mozilla-central/rev/2a6bac028754
https://hg.mozilla.org/mozilla-central/rev/3ad77c14180a
https://hg.mozilla.org/mozilla-central/rev/e9803b0d9c1d
https://hg.mozilla.org/mozilla-central/rev/55d1325436c1
Comment 13•4 years ago
|
||
These changes have been documented; see https://github.com/mdn/sprints/issues/3799#issuecomment-717309934 for the full details.
Let us know if you need anything else. Thanks!
Description
•