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

contenteditable-disabled #176

Closed
johanneswilm opened this issue Jun 26, 2018 · 23 comments · Fixed by #265
Closed

contenteditable-disabled #176

johanneswilm opened this issue Jun 26, 2018 · 23 comments · Fixed by #265

Comments

@johanneswilm
Copy link
Contributor

johanneswilm commented Jun 26, 2018

Hey, as of TPAC 2017, we have a resolution to create a contenteditable-disabled attribute [1] to be able to keyboard shortcuts and associated menu items from various context menus. It should work like this:

<div contenteditable="true" contenteditable-disabled="orderedList formatBold formatFontName">
  ...
</div> 

After consulting with developers of Substance.io, CKEditor, ProseMirror and TinyMCE, I would like to propose this list:

history // makes the element not be part of any browser-provided undo stack. 
inserHorizontalRule
orderedList
unorderedList
insertLink
insertLineBreak
insertParagraph
formatBold
formatItalic
formatUnderline
formatStrikeThrough
formatSuperscript
formatSubscript
formatJustify
formatDent // indent and outdent
formatSetBlockTextDirection
formatSetInlineTextDirection
formatBackColor
formatFontColor
formatFontName

There are some challenges with this:

  1. Clipboard actions - one cannot really avoid needing to offer these and accessing the clipboard programmatically is a security issue. So that means one will continue to have the native menu, which can be challenging when combining with an app-provided menu [2]. The solution there seems to be adding the ability to add custom actions to the native menus, but that will need to be dealt with in a separate issue. This was also briefly discussed at TPAC 2017.

  2. IE/Firefox by default add controls for tables, positioned items, images, and possibly more. It sounds like Firefox is working on removing these [3]. Should they not be removed, we would need to mention these here as well.

  3. It would be good to have a way to be able to disable all of these in one go. For example contenteditable-disabled="all"

[1] https://www.w3.org/2017/11/07-editing-minutes.html#resolution01

[2] ckeditor/ckeditor5#1107 and ProseMirror/prosemirror#7

[3] #171 (comment)

@Reinmar
Copy link

Reinmar commented Jun 26, 2018

The solution there seems to be adding the ability to add custom actions to the native menus, but that will need to be dealt with in a separate issue. This was also briefly discussed at TPAC 2017.

Assuming that we can't get access to undo/redo and clipboard's content, this indeed seems to be the solution. We can't completely hide native controls (those floating balloons) because we wouldn't have access to the clipboard, so adding our items to these menus seems to be the only way around.

However, until we get access to native menus, the situation won't be much better for us than it's now. We'll certainly disable all native actions we can, but we'll still have to fight with native menus (1 and 2) which don't leave space for our UIs.

So, it's a step forward and I'm all for it, but alone it won't help significantly.


It would be good to have a way to be able to disable all of these in one go. For example contenteditable-disable="all"

Such an option may not be future-proof. If we'll ever add more options to contenteditable-disable (e.g. typing or clipboard :D), then we may break applications which already use all. I'd use an explicit list.

@Reinmar
Copy link

Reinmar commented Jun 26, 2018

One thing that I don't understand is the relation of contenteditable-disable="<list of all options but history>" to contenteditable="plaintext-only" which standardisation was discussed in #162. These options seem to do pretty much the same thing.

@johanneswilm johanneswilm changed the title contenteditable-disable contenteditable-disabled Jun 26, 2018
@johanneswilm
Copy link
Contributor Author

@Reinmar It seems it will be fairly similar. With this you can turn on/off individual features though. So, far example, your editor may support bold but not italic. So you turn off italic, yet keep native bold buttons.

However, if your plan is to turn everything off and then reenable all features again in your own code and by adding new items to the native context menus, then it will be fairly similar with the exception of history, as you point out.

@masayuki-nakano
Copy link
Collaborator

Sounds odd attribute. How does it work with Document.designMode? Just available only with contenteditable?

@johanneswilm
Copy link
Contributor Author

@masayuki-nakano: This proposal grows out of the feedback from many JS editors over the years. We have a resolution to move forward with this initial proposal, but I don't think anyone is opposed to changing the attribute name or the mechanism by which the functionality is achieved as long as this is somehow provided.

I don't think that designmode been discussed so far. It would be interesting to hear from editors that use designmode about this. @Reinmar for CKEditor. We should also get someone from TinyMCE to look at this. And maybe we need to make a list of other current editors making use of design mode.

@Reinmar
Copy link

Reinmar commented Jun 28, 2018

I don't think that designmode been discussed so far. It would be interesting to hear from editors that use designmode about this. @Reinmar for CKEditor. We should also get someone from TinyMCE to look at this. And maybe we need to make a list of other current editors making use of design mode.

We don't use designMode. I don't know reasons for using it – how's it different from using contenteditable on <body>? If there's no difference, then I guess contenteditable-disabled (if we'll go with it) doesn't have to work with designMode.

@masayuki-nakano
Copy link
Collaborator

designMode works even with odd DOM tree such as documents which have two or more body elements, even after replacing body element etc. I can say, state of designMode can be managed with the document node. So, creating DOM API might be more reasonable than space separated list attribute.

@johanneswilm
Copy link
Contributor Author

johanneswilm commented Jun 28, 2018

DOM tree such as documents which have two or more body elements

What is the usecase of having more than one body element?

Looking at https://github.com/search?l=JavaScript&o=desc&q=designMode&s=updated&type=Repositories various Github search results, it indeeds doesn't look like there are production ready open source projects out there using designMode for anything where they cannot use contentEditable. TinyMCE seems to have moved away from designMode in 2011 [1]. So maybe we should email all non-open source projects to hear with them?

So, creating DOM API might be more reasonable than space separated list attribute.

If this is only for designMode, then that may not make much sense if it effectively is dead anyway. But we can discuss it again at the next TPAC. For now we have a resolution to use this space separated list attribute, which at least from the view of JS editors seems to fulfill its purpose.

[1] tinymce/tinymce@879c52a

@johanneswilm
Copy link
Contributor Author

@masayuki-nakano There seems to be cases where either now or in the past, one enabled designMode to get around some bug in Gecko. Have you seen this [1] before?

[1] https://github.com/vio-int/suitecrm2/blob/ec8676f42b9d614f4068839d4fe8f96892c8a4f7/include/javascript/mozaik/vendor/tinymce/tinymce/plugins/layer/plugin.js#L203

@masayuki-nakano
Copy link
Collaborator

DOM tree such as documents which have two or more body elements

What is the usecase of having more than one body element?

I don't think that it's not useful for any usual web apps. Such case may be used by attackers since not so tested by each vendor and usually causes crash bugs.

So maybe we should email all non-open source projects to hear with them?

I don't think so. If we keep designMode as part of web standards, new editing API should treat both designMode and contenteditable equally. Otherwise, each browser may need to check whether currently handling an input with designMode editor or contenteditable editor.

So, creating DOM API might be more reasonable than space separated list attribute.

If this is only for designMode,

I don't think so. Anyway, contenteditable requires JS to post the data. So, I have no idea that it's better to implement this with complicated new attribute of element.

There seems to be cases where either now or in the past, one enabled designMode to get around some bug in Gecko. Have you seen this [1] before?

I hope that it was a hack for our old bugs. Our HTMLEditor's event handler also handled wrong events whose target are not for the designMode editor.

@johanneswilm
Copy link
Contributor Author

I don't think so. If we keep designMode as part of web standards, new editing API should treat both designMode and contenteditable equally.

Well, in order to see whether there is any point in continuing with designMode, we should try to find out if anyone has a realworld usecase for it. I notice that CKEditor and TinyMCE used it in the past, but switched to contenteditable more than half a decade ago. One thing is keeping designMode around so that certain old websites don't break, but it's something else to add more features to it.

So I think we should find out: Is anyone using it for production, and if so: what is stopping them from moving to contenteditable? Having a document where the body element is exchanged is in itself not a usecase as long we don't know why anyone would do that, but maybe we can find a usecase where it is important to switch out the body element like that and not being able to add other attributes to the body element.

I hope that it was a hack for our old bugs.

Ok, I'll try to email them and ask what is going on there.

@Reinmar
Copy link

Reinmar commented Jul 3, 2018

So, creating DOM API might be more reasonable than space separated list attribute.

While I agree that a DOM API may be more flexible (especially, if in the future, the configuration of enabled/disabled features would get more complex), I feel that any discussion about whether it needs to support designMode is missing the point.

We have a coherent feedback from major rich-text editor authors. AFAIR none of us asked about doing anything with designMode. It's there, let it die. It shouldn't bother us. It shouldn't block anything. It's not something we need.

We should be focused on designing a consistent and useful API for the future. Not on standardising the old mistakes.

@johanneswilm
Copy link
Contributor Author

johanneswilm commented Jul 3, 2018

@Reinmar I generally agree that we should focus on the needs of actual editors. But we should always be open to the possibility that there are editors out there from whom we have not heard yet and who for some reason are using designMode. That's why I did some investigation into that on github, but the result there seems to be negative. So what is basically left are non-open source editors. If someone is able to find one of those we should also take them into consideration.

As for DOM API vs. flag list -- as with most such subjects, I don't have really strong opinions on one vs. the other. But there has been a great danger to such issues getting us distracted and then nothing happens. For now we have a resolution to have a space separated list of flags as an attribute. If Gecko would like to propose something else, I think we should have that as a topic at the TPAC meeting. Also with an API DOM, we will likely need to cover the same list of flags, so it's important that we don't stop completing the list while we are waiting for a discussion on an API DOM. Should we later go with an API DOM instead, we should be able to rapidly change the list of flags into a list of recognized keywords for a DOM API.

@Reinmar if you can think of good example uses of something beyond on/off for these features, then it's probably a good idea to write them down in a separate issue so that we can bring those up when we have the DOM API vs. flags list discussion at the F2F meeting.

@yoichio
Copy link

yoichio commented Jul 4, 2018

Aside from a way to point which editing functionality is disabled, I generally agree the idea of limited editable host.

@johanneswilm
Copy link
Contributor Author

TinyMCE has reported back that they also believe the above list of flags is complete!

@yosinch
Copy link

yosinch commented Oct 3, 2018

Do we really need to be done by declarative way, contenteditable-disabled?
How about imperative way Document#disableCommand(name) to disable command for current document to simplify designMode and nested case?

@johanneswilm
Copy link
Contributor Author

@yosinch That would not be ideal because it limits you to just one setting for all contenteditable elements on the same page, right? Also, we have still not found any usecase for designmode, so to me it feels like that taking designmode into consideration is not enough of a reason to give much worse functionality to contenteditable where we have known usecases for allowing different types of markup in different elements on the same page.

@Reinmar
Copy link

Reinmar commented Oct 3, 2018

Couldn't we call this method on editing hosts? I mean – it could be Element#disableCommand().

@johanneswilm
Copy link
Contributor Author

@Reinmar's suggestion would cover all usecases I have come across so far. I am not so sure of what it gives of added benefit when compared with the current solution though, unless we plan on expanding with more options than just enable/disable some day.

@gked
Copy link

gked commented Oct 3, 2018

While I generally agree to avoid declarative implementation for such behaviors, deviating from a well known and established pattern ofcontentEditable="..."may cause confusion among developers. Are there any concrete concerns against using declarative mechanism for contenteditable-disabled?

@Reinmar
Copy link

Reinmar commented Oct 3, 2018

There's execCommand() already and it was used to disable some features (like table editing handlers). I don't think this would be confusing to anyone.

From my point of view, the biggest problem with a declarative mechanism is that it may be limiting in the future if the configuration would need to become more complicated. Other than that, it seems ok.

@gked
Copy link

gked commented Oct 5, 2018

@Reinmar ok, fair enough.

@johanneswilm
Copy link
Contributor Author

I have tried with a first version that should provide what we discussed. It's basically using the DOMTokenList used in element.classList and and iframe.sandbox. But as we are disabling and we wanted to have a way to access a full overview of all supported token values, we would need to extend it in some way to allow for that (supports(token) only checks whether a given token is supported).

I don't feel support confident about webidl, so I am especially interested in reviews to the interface definitions.

@gked gked added the Agenda+ Agenda item to be inserted in the Editing TF meeting queue label Jan 8, 2020
@gked gked removed the Agenda+ Agenda item to be inserted in the Editing TF meeting queue label Mar 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants