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

Clarify association between elements and EditContext #40

Closed
dandclark opened this issue May 11, 2023 · 2 comments
Closed

Clarify association between elements and EditContext #40

dandclark opened this issue May 11, 2023 · 2 comments
Labels

Comments

@dandclark
Copy link
Contributor

@annevk said in #19 (comment):

Why is the association API on the element instance and not the EditContext instance?

The current design roughly follows contenteditable. Setting element.editContext = ...; parallels setting element.contentEditable = true;. But if there's a good reason to have the association API on the EditContext instead then we should consider that.

Also, we should formalize the "associated element" relationship in the spec with a <dfn>.

@masayuki-nakano
Copy link

At least, form controls (except <button>?) and void elements shouldn't become editable since it's impossible to update the content with modifying the DOM tree. And also from a maintainer of builtin editor of Gecko point of view, contenteditable with such elements causes unexpected crash and security bugs a lot. Therefore, it might be better to apply the list to contenteditable too.

On the other hand, the DOM API allows to put any elements as children as any elements. Therefore, there are some tricky cases even if the spec restrict the elements can work with EditContext. E.g., an element associated with an EditContext can be moved under <select> and <option>. I'd like the spec defines the cases as treated as that EditContext is not set.

@dandclark
Copy link
Contributor Author

This was discussed in today's WG meeting. We resolved to keep the API on HTMLElement.

[08:18] <whsieh_> johanneswilm: onto #40
[08:19] <whsieh_> dandclark: works by introducing new HTML attr on Element
[08:19] <whsieh_> dandclark: should we consider alt. where we just have API on EC?
[08:20] <whsieh_> dandclark: (e.g. EC.attach())
[08:20] <whsieh_> dandclark: kind of like current API shape since it mirrors contentEditable
[08:20] q+
[08:20] <whsieh_> dandclark: devs will be used to that pattern
[08:20] <whsieh_> dandclark: maybe that outweighs downside of polluting HTML element API surface
[08:21] <whsieh_> smaug: also like current API shape
[08:21] <whsieh_> smaug: nice because the element naturally keeps EC alive
[08:21] <whsieh_> smaug: re: only working on subset of elements, there is precedent for attachShadow()
[08:21] q+
[08:22] <whsieh_> johanneswilm: this isn't an API that you quickly hack together in dev console
[08:22] +1 to aligning with precedent like attachShadow
[08:22] <whsieh_> johanneswilm: in practice, won't make much difference because barrier to adoption is relatively high
[08:23] <whsieh_> dandclark: lifetime of EC makes a lot of sense
[08:24] <whsieh_> whsieh_: I'm okay either way. above arguments make sense to me
[08:24] == alexk [~alexk@1d899584.public.cloak] has joined #editing
[08:24] <whsieh_> johanneswilm: resolution: group is okay with the current behavior, and we'll sync up with Anne

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

No branches or pull requests

2 participants