Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should beforeinput be fired when execCommand is called by addons? #91

Closed
masayuki-nakano opened this issue Jan 9, 2019 · 7 comments
Closed

Comments

@masayuki-nakano
Copy link

If there is an addon, which implements same shortcut keys as Word with execCommand (e.g., Ctrl+U to toggle underline). Then, with current draft, browsers won't dispatch beforeinput event for the change. However, for the purpose of beforeinput, web apps must want to know the change for canceling. So, shouldn't beforeinput be fired when execCommand is called by addons?

@johanneswilm
Copy link
Contributor

No, beforeinput should only be fired upon user input. And execCommand should generally not be used for new features - see the notice at the start of the execCommand draft spec.

However, if a addon really really wants to use execCommand, it can also manually fire the beforeinput event. At least I think that should work.

We should move this issue to https://github.com/w3c/editing as that is where the execCommand spec draft lives. The Input event should not refer to execCommand because that spec has no official standing.

@masayuki-nakano
Copy link
Author

No, beforeinput should only be fired upon user input.

From the point of the view of web apps (and their developers), isn't such execCommand execution exactly same as build-in function of browser (both are kicked by user input)? For example, somebody may create an addon to map Ctrl+B as toggling bold-style on Firefox like Chrome. Then, don't web apps need to ban the execCommand from addon if they do so for Ctrl+B on Chrome?

it can also manually fire the beforeinput event.

I don't think that it's possible scenario. Some addon developers love hacky approach. I.e., they may write addon even if something they want to do should be done in browser itself (i.e., they write addons rather than contributing browsers).

The Input event should not refer to execCommand because that spec has no official standing.

I completely disagree with it. Not dispatching beforeinput for execCommand or similar API call from JS is really important behavior for browsers. At least, even as informative comment, the spec should mention this behavior because even though they are non-standardized API, they have been there for a long time. Implementations really require clear definition even related with non-standardized API.

@rniwa
Copy link

rniwa commented Jan 11, 2019

Not dispatching beforeinput for execCommand or similar API call from JS is really important behavior for browsers. At least, even as informative comment, the spec should mention this behavior because even though they are non-standardized API, they have been there for a long time. Implementations really require clear definition even related with non-standardized API.

Yeah, regardless of whether we dispatch the event or not, the spec should absolutely clarify the behavior of it either way. It makes no sense not to specify the behavior just because the API doesn't have a clear spec given how much of the Web currently depends on it.

@johanneswilm
Copy link
Contributor

For example, somebody may create an addon to map Ctrl+B as toggling bold-style on Firefox like Chrome.

Someone else, writing equally hacky JS code, may create a addon that always replaces all the "l" with "1" at random intervals without any human interaction to annoy the user. In that case, there should be no beforeinput-event, right? So wouldn't the best place to put the trigger of the beforeinput event be in the JS code of the plugin?

Yeah, regardless of whether we dispatch the event or not, the spec should absolutely clarify the behavior of it either way. It makes no sense not to specify the behavior just because the API doesn't have a clear spec given how much of the Web currently depends on it.

That is a good point, but I think this should then be in the unofficial execCommand spec draft. That document is defacto being used by some browsers (for example Firefox added clipboard-related things there a few years ago), and I would argue that this is where it makes most sense. As it currently is, the Input Events spec, as I read it, is written so that there are no beforeinput events if the DOM change is caused by JS code and not direct human interaction. So without execCommand being mentioned, it also covers that. If we decide that we want to trigger the event, I think that should go into the execCommand spec.

@johanneswilm
Copy link
Contributor

I don't think that it's possible scenario. Some addon developers love hacky approach. I.e., they may write addon even if something they want to do should be done in browser itself (i.e., they write addons rather than contributing browsers).

I think we are talking about different things here. I am saying that if the person writing this addon is writing a line of code to trigger execCommand, the same developer can just before it include a line of code that triggers a beforeinput event from JS code on that element.

Just as a sidenote: I know I have said this many times, but we have to be clear that any such addon will break on a large number of sites. Even something as simple as what you mention (ctrl-b on firefox for strong) will likely break quite a number of sites, because the editors have all been programmed to check which browser they are on and so will work slightly differently on each browser and so the editor doesn't expect that there can be an execCommand('bold') on Firefox and so it just crashes. The one addon that we know about that has done something like that in the past, Grammarly, is now doing something more sophisticated where it hides the original text input field and instead shows its own on top of it and copies text between the two. And it's written to recognize specific sites where it then actually works by hooking into the JS editor running on that site. Grammarly still crashes quite a bit, but it's better. Why is this important? Because the entire discussion on this is somewhat besides what in reality is out there and what in reality works or can be expected to work. And it's not really fixable unless you fix all of execCommand in the browsers.

Can we please move this discussion over to https://github.com/w3c/editing/ because that's where execCommand lives.

@scottfr
Copy link
Contributor

scottfr commented Dec 11, 2019

We build a Chrome extension that can trigger paste events within content editable fields. The lack of the BeforeInput events as part of execCommand creates the following issue.

The order of events for a user generated paste action in Chrome is:

  • Paste
  • BeforeInput
  • Input

If we need to generate the BeforeInput ourselves separate from the execCommand, we get the following order of events:

  • BeforeInput [manually triggered]
  • Paste [execCommand triggered]
  • Input [execCommand triggered]

This has the potential to break applications that depend on the specific order.

An example is it breaks the Trix 1.2.2 editor (https://github.com/basecamp/trix) which inputs text on both the Paste event and the BeforeInput event. They call preventDefault() on Paste event so when events follow the correct order, things work correctly. However, if we have the BeforeInput event before the paste event, we get double insertions.

@johanneswilm
Copy link
Contributor

Resolution call 2020-01-10: close in favor of w3c/editing#200

xeonchen pushed a commit to xeonchen/gecko that referenced this issue Mar 30, 2020
….execCommand()` is called by addon r=smaug

Discussion in
* w3c/input-events#91
* w3c/editing#200

Web developers must want to handle `beforeinput` events which are caused by
addons because addon's behavior is exactly same as default action of browser
from point of view of web apps.

Differential Revision: https://phabricator.services.mozilla.com/D68528
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Mar 30, 2020
….execCommand()` is called by addon r=smaug

Discussion in
* w3c/input-events#91
* w3c/editing#200

Web developers must want to handle `beforeinput` events which are caused by
addons because addon's behavior is exactly same as default action of browser
from point of view of web apps.

Differential Revision: https://phabricator.services.mozilla.com/D68528

--HG--
extra : moz-landing-system : lando
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Mar 31, 2020
….execCommand()` is called by addon r=smaug

Discussion in
* w3c/input-events#91
* w3c/editing#200

Web developers must want to handle `beforeinput` events which are caused by
addons because addon's behavior is exactly same as default action of browser
from point of view of web apps.

Differential Revision: https://phabricator.services.mozilla.com/D68528

UltraBlame original commit: 684fdad241529b919670d21d860edc094512946f
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Mar 31, 2020
….execCommand()` is called by addon r=smaug

Discussion in
* w3c/input-events#91
* w3c/editing#200

Web developers must want to handle `beforeinput` events which are caused by
addons because addon's behavior is exactly same as default action of browser
from point of view of web apps.

Differential Revision: https://phabricator.services.mozilla.com/D68528

UltraBlame original commit: 684fdad241529b919670d21d860edc094512946f
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Mar 31, 2020
….execCommand()` is called by addon r=smaug

Discussion in
* w3c/input-events#91
* w3c/editing#200

Web developers must want to handle `beforeinput` events which are caused by
addons because addon's behavior is exactly same as default action of browser
from point of view of web apps.

Differential Revision: https://phabricator.services.mozilla.com/D68528

UltraBlame original commit: 684fdad241529b919670d21d860edc094512946f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants