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 execCommand be spec'd to do nothing in cE=typing? #33

Closed
BenjamP opened this issue Dec 15, 2014 · 26 comments
Closed

Should execCommand be spec'd to do nothing in cE=typing? #33

BenjamP opened this issue Dec 15, 2014 · 26 comments
Milestone

Comments

@BenjamP
Copy link

BenjamP commented Dec 15, 2014

See mail thread at http://lists.w3.org/Archives/Public/public-editing-tf/2014Dec/0101.html

contentfEditable=typing should not call execCommand behavior by default, but should it work if javascript calls the APIs?

@BenjamP
Copy link
Author

BenjamP commented Dec 15, 2014

It seems like someone will want it, so I don’t think there’s a strong enough case to explicitly remove it. If you don't want it, just don't call the API.

@fredck
Copy link

fredck commented Dec 16, 2014

I was wondering if UAs will have to introduce a whole new bunch of code to be able to handle execCommand on cE=typing properly. I mean, there may be differences in things like selection that will force UAs to handle things differently. Some commands, like "delete", are even supposed to not be handled by UAs in cE=typing.

The above, other than eventually slowing down the adoption of cE=typing, will also push forward one of the biggest problems with cE=true, where commands have no specs.

  • Option 1: explicitly declare that execCommand is to be supported, in the new cE=typing specs.
  • Option 2: explicitly declare that execCommand must have no behavior, in the new cE=typing specs.
  • Option 3: completely ignore execCommand and see this as an independent API out of the contenteditable specs and scope.

I tend to like "Option 3".

@teleclimber
Copy link

I'd vote for Option 2, with the anticipation that editor devs will create JS libraries to fill those roles. I think this is a place where JS libs can be very effective.

@fredck
Copy link

fredck commented Dec 16, 2014

@teleclimber, I agree... should re-phrase my opinion: I tend to like "Option 3", but I think "Option 2" is the most realistic way for it.

@BenjamP
Copy link
Author

BenjamP commented Dec 16, 2014

I want to make sure we’re all on the same page. The idea of contentEditable=’typing’ is to get rid of all of the default input behavior of browsers on user interaction (other than typing characters). So enter key, delete key, keyboard shortcuts, drag and drop, browser toolbars, etc have no effect besides firing device independent events. However, if a site calls a javascript API like appendChild or execCommand it should work. Why would we want to disable any javascript from working exactly as it works today? We shouldn’t need to update the selection normalization in those commands either. If script calls them it needs to deal with any fallout.

@teleclimber
Copy link

I'm OK with Option 3 if that's how it works out.

@fredck
Copy link

fredck commented Dec 16, 2014

I'm fine "Option 3" and I see that the IE team likes it. It would be wonderful to have it confirmed by other browser vendors.

@kojiishi
Copy link

Take this is my personal opinion at this point, but I'm still in favor to disable it. I'm open to how to disable it, from ignoring to throwing.

I have no idea how Gecko/IE implement execCommand(), but I'm guessing there will be a lot of work for all UAs. Consider JS sets the selection to an element with display:none. Or, anchorNode pointing to cE=false. Since at least WebKit/Blink used to normalize such selections when they're set, execCommand has never received such selections as input. What execCommand should do, and whether queryCommandEnabled should return false or not for such selections are completely new behaviors.

And the history tells me that if we do it without specs and were not interoperable, someone will file bug reports saying "I consider this UA doing that way is the right thing." Isn't that what we're trying to get out from?

We had a history of our non-interoprability troubled developers and users so much. Maybe it's my false fear, it may be quite easy to upgrade all execCommands to new behaviors, but we could allow it anytime if we want to. Without concrete needs expressed, I'd prefer to start from the minimum and prioritize us on interoperability.

@BenjamP
Copy link
Author

BenjamP commented Dec 31, 2014

It may not work interoperably, but why spec it to not work? I want to be really careful to solve the right problems here instead of broadening out too much. We should spec things that have the most impact first, in my opinion. Can we just ignore these APIs for now?

@kojiishi
Copy link

I think -- well, I'm not familiar with the history, sorry in advance if I'm wrong, but -- the idea of contentEditable='typing' is that it's the best method to make editing interoperable. If we ignore this for now, and sites started building on top of one implementation within contentEdiable=typing, how could we make it interoperable later on?

If there were specific commands developers want to use within contentEditable=typing, I'd be happy to allow them. But again I'd like such commands have good interoperability, I'd like the list of commands white listed and spec'ed so that browser developers can make them interoperable.

Well, I understand spec'ing them is hard, but not allowing is quite easy. Do you have specific reasons you want to allow it, and idea how to make it interoperable later on?

@KenjiBaheux
Copy link

I'm siding with Koji on this one.

Unless there is something that one can not do without execCommands, we should not allow them in the ce=typing world given that (IIUC) it tries to avoid the sins of the past. If the minimal viable product needs some of the execCommands then Koji's suggestion of a whitelist with spec-ed behaviors(*) in these new ce contexts seems reasonable.

Furthermore, if we keep them around in these new ce contexts as-is, we would loose some on the opportunity to deprecate and/or supersede these non interoperable buggers, right?

*: if these chosen few are known to have interoperability issues.

@rniwa
Copy link
Contributor

rniwa commented Jan 5, 2015

Like Ben, I don't understand the point of disabling execCommand either since execCommand is a legacy API and there is literally no hope for removing it in any foreseeable future.

Disallowing execCommand completely is, as a matter of fact, non-trivial if we allow a selection that crosses editing boundaries to exist (i.e. if we don't force UA to normalize such a selection).

@kojiishi
Copy link

kojiishi commented Jan 5, 2015

Just to be clear, I wish to disable only when the selection is within cE=typing so that we can assure that everything within cE=typing is interoperable, but keep it undefined for other cases. Are we in sync on this point?

If you're concerned about when the selection contains cE=true, cE=false, and cE=typing, it's not too hard to work it out; either "at least one" or "if anchor is," either way, it's an edge case we don't need to be really interoperable.

If we're on the same page but still do not agree, I'd like to ask how we'd make cE=typing interoperable when execCommand is allowed to run. If there were any other good way to assure interoperability, I'd be happy to discuss that.

@rniwa
Copy link
Contributor

rniwa commented Jan 5, 2015

Let us first observe that allowing execCommand doesn't cause any interoperability issues as long as authors don't manually call execCommand since cE=typing doesn't trigger any execCommand by user actions such as inserting a new paragraph, or deleting the selection, except inserting a new character.

Now notice that even if we "disallowed" execCommand inside cE=typing, authors can always override the contenteditable attribute to "true" temporarily and call execCommand with arbitrary arguments. So there is no way for us to "prevent" authors from having interoperability issues if they decide to use execCommand. And they can always decide to use execCommand at any moment because there is nothing that prevents authors from overriding contenteditable attribute.

@kojiishi
Copy link

kojiishi commented Jan 5, 2015

I think I understand where we misunderstand each other. I was talking about when developers wanted to use execCommand in a new code base using cE=typing, there's nothing that prevents or even discourage doing so. You worry that, if we prohibit that, we have to implement all commands to fail, correct?

How about this:

When the selections contain contentEditable=typing, execCommand, queryCommand, (etc., fill in others) may not work as they do in other elements, or may even fail to execute. These methods are not interoperable nor guaranteed to work against contentEditable=typing, and their uses are strongly discouraged.

I agree that the cost to make sure they fail is worthless, but without spec saying anything, I think natural expectations are these methods work well, and we may end up seeing bugs wanting execCommand to work the same way as in, say, IE in cE=typing. I'd like to avoid such expectations.

I'd like to prevent both from happening, and I wish the spec to have text to clarify that.

@fredck
Copy link

fredck commented Jan 5, 2015

@kojiishi, @rniwa, I think the following summarizes what both of you are looking for (from my previous comment):

Option 3: completely ignore execCommand and see this as an independent API out of the contenteditable specs and scope.

@NorbertLindenberg
Copy link

I agree with Koji that a warning would be appropriate to avoid expectations that implementations can't meet.

@BenjamP
Copy link
Author

BenjamP commented Jan 22, 2015

I continue to like @fredck's option 3.

@NorbertLindenberg- if a browser wants to throw warnings in dev tools, that seems fine, but I don't think we need to spec it.

@BenjamP BenjamP modified the milestone: V1.1 Jan 28, 2015
@NorbertLindenberg
Copy link

What Koji proposed on 2015-01-05 and I supported on 2015-01-07 is a warning in the spec text, not warnings produced by any implementation.

@johanneswilm
Copy link
Contributor

Hey guys, can we get some kind of resolution on this? Everyone seems to agree that we don't want to spec out the different execCommand methods right now and we might not ever support any of them inside cE=typing.

It's always easier to add functionality later on rather than remove it, and I don't think the proposals here really diverge much. About we add this text to the spec text under contenteditable="typing":

execCommand and queryCommand should not affect elements that are editable because they or one of their ancestors has their contenteditable attribute set to "typing".

If we want to be more permissive at a later stage, we can always change that with a white list, etc. . We just need to have a solid minimum now.

@fredck
Copy link

fredck commented Apr 23, 2015

Actually, I think that the overall consensus is to this:

Option 3: (1) completely ignore execCommand and see this as an independent API out of the contenteditable specs and scope.

... added by (2) an eventual warning in the specs that underlines the above.

I still don't feel that (2) is necessary, but it should not hurt.

@johanneswilm
Copy link
Contributor

@fredck Ok, but currently, when an element has cE enabled, execCommand and queryCommand are also enabled on that element automatically, right? So you need to say whether this is also the case for cE=typing or not.

Do I understand you correctly that you think that cE=typing should behave the same way as cE currently does, but that we should eventually write a separate spec for how execCommand works?

@johanneswilm
Copy link
Contributor

execCommand should only work in designMode or when contentEditable is enabled, if I read this correctly: https://developer.mozilla.org/en/docs/Rich-Text_Editing_in_Mozilla right?

@johanneswilm
Copy link
Contributor

@fredck Let me add the warning now, so we can get on with this and close this issue. This is only an editor's draft, so we can change it still.

@fredck
Copy link

fredck commented Apr 23, 2015

Do I understand you correctly that you think that cE=typing should behave the same way as cE currently does, but that we should eventually write a separate spec for how execCommand works?

Yes, exactly.

IMHO there is no relation between cE and execCommand, except that those are feature related, usually, to DOM manipulation for the scope of editing. Still they can (and should) leave totally isolated.

In other words, execCommand should not be a feature of cE, but a document feature... and sincerely, it should not dependent on cE at all... it should just take the current selection and execute the requested command on it, period. (I mean, even in non cE selections)

If we take this approach, we'll have less complications for now into the cE=typing specs.

@johanneswilm
Copy link
Contributor

@fredck Ok, that makes sense. I've added the notice and with that I assume this ticket has been resovled. Should someone think that it has not been reoslved, please feel free to reopen the ticket.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants