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.

Sign up for GitHub

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

Jump to bottom

Agenda 2022-07-21 #8

Closed
jgraham opened this issue Jul 20, 2022 · 10 comments
Closed

Agenda 2022-07-21 #8

jgraham opened this issue Jul 20, 2022 · 10 comments

Comments

@jgraham
Copy link
Contributor

jgraham commented Jul 20, 2022 •
edited

@masayuki-nakano
Copy link

masayuki-nakano commented Jul 22, 2022 •
edited

I added (will add) some tentative WPTs for contenteditable edge cases:

Tests cloning attributes except id attribute at splitting an element and merging attributes at joining an element

Tests

Overview

Chrome and Safari clones id attribute. This is odd from point of view of its meaning in HTML, but this makes sense for keeping the style. However, this may cause that document.getElementById() returns different element after splitting an element with Enter key press or something. So, I'd like to suggest that id attribute should be kept in same element, but not cloned to new element. Then, document.getElementById() keeps returning same element.

Background

Firefox currently creates left element at splitting, and Chrome/Safari creates right element instead. This is what I'm currently working on to make Firefox behave same as Chrome/Safari. However, Gecko does not clone id attribute at splitting paragraphs. Therefore, after aligning the split/join node direction of Firefox to the other browsers, this is a potential risk of new web-compat issue because getElementById returns first element which has the specified id value and the other browsers' behavior does not make sense at least from the point of view of HTML semantics.

Tests inserting paragraph or line break in <span contenteditable>

Tests

Overview

Both Chrome and Safari changes how to handle insertParagraph (pressing Enter) and insertLineBreak (pressing Shift-Enter in Firefox/Chrome or Control-Enter in Safari) if display value of the <span contenteditable> is changed. If it's styled as "block", Chrome and Safari inserts <div> or <p> into the <span contenteditable> at insertParagraph. This is completely odd and invalid structure. I think that insertParagraph should also work as insertLineBreak in an editing host which cannot have <div> or <p> element as a child. I.e., browsers should insert <br> or a preformatted linefeed. (For making the builtin editors keep faster, I think that it's enough to check only parent element before inserting <div> or <p> element, i.e., if web apps have already created <div> in <span>, insertParagraph in the odd <div> should work as usual.)

Background

Some developers (but not so many) have reported some bugs of <span contenteditable> behavior so that this case may be useful for some developers if it works cleanly between browsers, so such inline editing host may not be used so widely even currently, the reason could be caused by the odd behavior of Chrome and Safari. I know builtin editors of all browsers create odd sub-list with execCommand("insertOrderedList") and execCommand("insertUnorderedList"). However, this is completely different level issue. This issue can happen with users' operation in contenteditable.

Tests inserting paragraph and delete in elements outside <body>

Tests

Overview

Firefox did not allowed users to modify anything outside <body> element for avoiding to delete special elements such as <head>, <body>, <style>, <link>, <body>, etc. However, this caused a web-compat issue. Therefore, browsers need to allow editing outside <body>. Instead, I'm currently thinking that for making the rules simple and safe, I'd suggest the following rules:

  • insertParagraph in usual elements (meaning usually appear in <body> and visible by default) should work even if it's outside <body>.
  • insertParagraph in special elements which are invisible by default such as <script>, <style>, <title> should not split them.
  • insertParagraph/insertLineBreak in special elements which cannot have any elements such as <script>, <style>, <title> should do nothing if their white-space does not preserve linefeed (e.g., normal). Otherwise, i.e., white-space preserves linefeed, should insert a linefeed (and additional one if it creates an empty last line) (e.g., pre, pre-line, pre-wrap).
  • delete/forwarddelete should not join special elements such as <script>, <style>, <title> with another element even if same elements. This rule protects the structure of the special elements.
  • delete/forwarddelete should join usual elements even if left/right elements cross <body> or <head> boundary, but don't delete <body> and <head> for protecting the document structure.
  • delete/forwarddelete should not delete invisible elements in <head> element at joining 2 visible usual elements in <head> element or crossing the boundary of <head>. (<link>, <base>, <script>, <style>, <template>, <noscript>, <meta>, <title>. Perhaps, they should be treated as not deletable node even in <body> makes things simpler.)
  • delete/forwarddelete should not unwrap paragraph nor list element which is a child of <html> or <head> to avoid making the parent children messy.
    These rules are restriction of the edge cases. Therefore, I think that browsers can conform to these rules simple patches.

Background

This is completely odd case since web apps put editable elements outside <body> element. However, according to bug reports for Firefox, this is used for placeholder of temporary use at least (e.g., for getting pasted into invisible <div> element outside <body> and sanitize it and move into focused editor). Therefore, I don't think browser developers should work on this immediately, but there should be rough design of the behavior which are agreed by all browser vendors.

@johanneswilm
Copy link

johanneswilm commented Aug 4, 2022 •
edited

Tests inserting paragraph and delete in elements outside

Is there any way to do this without using execCommand? Can users trigger these kinds of changes outside of <body> by using mouse or keyboard or similar?

@masayuki-nakano
Copy link

masayuki-nakano commented Aug 5, 2022 •
edited

Tests inserting paragraph and delete in elements outside

Is there any way to do this without using execCommand? Can users trigger these kinds of changes outside of <body> by using mouse or keyboard or similar?

Yes, the tests uses the test driver instead of execCommand.

I understand that it should not occur in theory (i.e., web apps should avoid to put editable elements outside <body>, but it's unfortunately possible, and I really want the direction how to support the cases when we need to change the behavior of Gecko for broken web apps. In other words, I do not request the other browser vendors to change for these rare cases immediately, but I want the agreement between all browser vendors.

@johanneswilm
Copy link

@masayuki-nakano I understand this boils down to just one issue [1]. And that issue was about a software called "Woltlab Burning Board 4.0" that reached end of life in 2017 [2]. The version of CKEditor 4 that software used was from 2013 and the issue cannot be reproduced in any current versions of CKEditor 4 or 5.

While there is nothing wrong with making browsers interoperable in this area, one needs to be aware that it's not an issue for any current JS editor.

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1722535
[2] https://www.woltlab.com/community/thread/259873-burning-board-4-0-end-of-life/

@masayuki-nakano
Copy link

@johanneswilm Thank you for the information, it's really helpful for handling reported bugs.

On the other hand, it cannot be a reason to ignore the situation, "current known editors do not use it". Once we'll get a report of broken web app (including newly developing apps), anyway, we need to extend the ability of editing outside <body> elements, and in such cases, I'll have a trouble that how to make Gecko behave, which has never been discussed in any specs (nor WPTs).

If all browsers stopped supporting of editable ability outside <body>, it'd make browsers and the spec simpler, but it's impossible due to breaking (at least) legacy web apps whose instances still run in somewhere.

@johanneswilm
Copy link

@masayuki-nakano I am not against doing something about it. However, if you change any of the editing behavior of the browser, you will always risk breaking other legacy JS editors that are deployed beyond their end of life. In this case, the website owners could have updated to a later version of CKEditor 4 (or upgraded to CKEditor 5), which are both available as open source software, but chose not to.

So if we want to touch execCommand at all and accept the risk of breaking older apps, I would propose simplifying and just say there is no editing allowed outside of <body> also in Chrome/Webkit. Are there any usecases of being able to do this that we know of where this is actually needed?

@masayuki-nakano
Copy link

@johanneswilm Right, any changes have the risk of breaking backward compatibility. Therefore, I believe that disabling any editing outside <body> is really risky, and it'd actually break the old CKEditor users. Therefore, my suggestion is, the editing keeps working as usual even outside <body>. However, for avoiding to handle unrealistic cases (and causes really odd result), browsers should stop deleting, joining and splitting <html>, <body>, <head>, <script>, <style> and some other elements which are typically appear in <head>. In other words, if I makes Gecko support editing outside <body> completely, it blocks me that how to treat these special elements. I think that this approach is the safest and requires only minimum change for Chrome and Safari, but should not affect to existing web apps.

The expectations in my posted WPTs conforms to this idea, and Chrome and Safari do not confirm only to the special elements' handling.

@johanneswilm
Copy link

@masayuki-nakano Right, it will break a nine year old version of CKEditor that reached end of life many years ago. But so will doing any change at all to anything execCommand and contenteditable-related, including the above proposals of changing how paragraph splitting is done and also the proposal for limiting using execCommand outside of <body>.

I think we have to decide between one of these three ways forward:

  • We decide that we cannot change anything about execCommand because it will break old web apps. In that case, everything execCommand related has to stay exactly as it is now, including differences between browsers as JS editors contain browser specific code.

  • Alternatively, if we don't think breaking very old things and hardly used features is an issue, then we should simplify (which in this case would mean remove the ability to edit outside of <body>).

  • A third option could be to make a larger investigation of all the main JS editors that have been released over the past decade that might potentially still be in use somewhere and check with every proposed change whether any of them will break. Anything that breaks at least one of them, cannot be done. This would open for implementing your above proposal if, and only if, we find that it breaks no editor.

Or maybe you/someone else here can think of a fourth way?

@masayuki-nakano
Copy link

@johanneswilm

* We decide that we cannot change anything about `execCommand` because it will break old web apps. In that case, everything `execCommand` related has to stay exactly as it is now, including differences between browsers as JS editors contain browser specific code.

It's not realistic approach for Mozilla because some web apps depend on the behavior in Chrome and/or Safari, and Firefox may need to align the behavior to them (as you're thinking, this sometimes breaks other apps, but not standardized things make 2nd or lower market share browsers work difficult😭). Additionally, execCommand handling is also used as the path to handle user's input (at least in Firefox and Chrome, I guess Safari too). Therefore, execCommand behavior may be changed by changes for different purpose. Therefore, freezing (only) the execCommand behavior of editor makes browsers have 2 different paths so that increase the maintenance cost and binary size, and freezing the behavior requires a lot of WPT for detecting regressions. I think nobody wants to work on it.

* Alternatively, if we don't think breaking very old things and hardly used features is an issue, then we should simplify (which in this case would mean remove the ability to edit outside of `<body>`).

My suggestion is this approach. I suggest that removing only partial behavior which must be not used by web apps in the wild. Disabling all ability to edit outside <body> is too risky.

* A third option could be to make a larger investigation of all the main JS editors that have been released over the past decade that might potentially still be in use somewhere and check with every proposed change whether any of them will break. Anything that breaks at least one of them, cannot be done. This would open for implementing your above proposal if, and only if, we find that it breaks no editor.

In my experience, not only major (rich) editor libraries are used by web developers. We've gotten bug reports which were probably found by experiments or at developing web apps. Especially, nobody cannot know web apps which are used only in a company/organization, so I think that researching only "well known" and "public" products do not make sense for considering the reasonable behavior (of course, the result is useful if there is).

@johanneswilm
Copy link

In my experience, not only major (rich) editor libraries are used by web developers. We've gotten bug reports which were probably found by experiments or at developing web apps.

Ok maybe, but the bug report that the last issue depends on is from one of those well-known editors. It generally takes years to program a well-functioning JS editor, which is why there aren't that many self-made ones out there used on production-level sites.

This now looks like a discussion on what behavior to standardize on, so I will file a ticket in the editing repository and put it on the agenda for the call on Thursday.

@johanneswilm johanneswilm mentioned this issue Aug 9, 2022
Open
@hsinyi hsinyi mentioned this issue Aug 9, 2022
Closed
@jgraham jgraham closed this as completed Oct 19, 2022
@jgraham jgraham mentioned this issue Oct 20, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@jgraham @johanneswilm @masayuki-nakano