search

w3c / input-events

Input Events
https://w3c.github.io/input-events/
Other
24 stars 16 forks source link

Clarify what `getTargetRanges` should return when all content in an inline element is removed #158

Open masayuki-nakano opened 1 week ago

masayuki-nakano commented 1 week ago

I think that this is the first step to define getTargetRanges behavior. (spinning off from #146)

  1. Backspace when <p>ab<span>c[]</span>de</p>
  2. Backspace when <p>ab<span>c</span>[]de</p>
  3. Delete when <p>ab<span>[]c</span>de</p>
  4. Delete when <p>ab[]<span>c</span>de</p>

In all cases, browsers delete <span> rather than only "c". According to input-events-get-target-ranges-backspace.tentative.html and input-events-get-target-ranges-forwarddelete.tentative.html, browsers (try to) return:

  1. <p>ab<span>[c]</span>de</p>
  2. <p>ab<span>[c]</span>de</p>
  3. <p>ab<span>[c]</span>de</p>
  4. <p>ab<span>[c]</span>de</p>

I think that current behavior is better for implementing same behavior in any browsers because if browsers computes the range to delete, they just need to shrink down the range to minimize the range in leaf nodes.

I think that under current definition, the <span> should be included, i.e., <p>ab{<span>c</span>}de</p> is the expected one. However, it's harder to extend the range from performance point of view. Additionally, even if an inline element which gives a style like <b> is deleted from the DOM, new typing text could preserve the style. In such case, including parent elements may make web apps confused. Therefore, I believe that it's the best to shrink the target range as far as possible.

(Although out of scope of this issue, think Backspace when <p>abc</p><p>[]def<br>ghi</p> case. All browsers tries to return a range around </p><p>. However, the first text node def and the <br> in the second paragraph will be moved to the first paragraph, therefore, in strictly conforming to current definition, <p>abc[</p><p>def<br>}ghi</p> should be included, but def will be preserved so that the range is not meaningful for web apps. So, anyway the spec should clarify getTargetRanges behavior, and the my recommending behavior makes the rule of this kind of situations simpler too. Since the first line in the second <p> may be wrapped with some inline elements. Then, browsers do not need to consider whether the range ends before inline elements or at first text.)

masayuki-nakano commented 1 week ago

Oh, I filed https://github.com/w3c/input-events/issues/112 on 2020...

masayuki-nakano commented 1 week ago

As far as I've checked for Backspace cases, <p>a<span>bc[]</span>d</p> and <p>a<span>bc</span>[]d</p> make getTargetRanges() return <p>a<span>b[c]</span>d</p>. So, shrinking the range into text node or around leaf node (e.g., <br>, <img>) as far as possible may make sense.

masayuki-nakano commented 1 week ago

Backspace when <p>a<span><img></span>[]b</p>, all browsers return <p>a<span>{<img>}</span>b</p> (although Firefox does not delete the <span>, this is a bug).

Backspace when <p>a<span><img><img></span>[]b</p>, all browsers return <p>a<span><img>{<img>}</span>b</p>. So, shrinking target range around leaf node is reasonable for defining the behavior simply.

masayuki-nakano commented 1 week ago

(Once it's agreed or there should be WPT for considering the final answer, I'll write WPT. However, I'd like to know the opinions before writing tests.)

johanneswilm commented 1 week ago

Also these proposals are general proposals for how contenteditable/execCommand should work, given that the other browser makers are not willing to have divergent target ranges between contenteditable and EditContext, correct?

johanneswilm commented 1 week ago

According to input-events-get-target-ranges-backspace.tentative.html and input-events-get-target-ranges-forwarddelete.tentative.html, browsers (try to) return

Those two tentative tests test things that are not specified by any specification, correct? Therefore we cannot use these as source of truth. I don't necessarily disagree with browsers behaving this way, but given that the browsers will depend on platform editing behavior, and such behavior may change in the future, I don't see how we can specify that.

masayuki-nakano commented 1 week ago

Also these proposals are general proposals for how contenteditable/execCommand should work, given that the other browser makers are not willing to have divergent target ranges between contenteditable and EditContext, correct?

Right, first, getTargetRanges should be defined deeper to make getting same result in EditContext because in contenteditable, the result should represent the result on the browser, but in EditContext, there is no actual result, but web app developers want "recommendation" to delete the range. My suggestion is, browsers should return minimized range to delete. Then, web app developers can do more things if their app should work as so.

(err, but I don't suggest web browsers should change/align the editing result. I just want the normalization rules of target ranges to represent same intentions of browsers with same StaticRange.)

According to input-events-get-target-ranges-backspace.tentative.html and input-events-get-target-ranges-forwarddelete.tentative.html, browsers (try to) return

Those two tentative tests test things that are not specified by any specification, correct?

I wrote the expectations to conform to the old definition of getTargetRnages(). IIRC, that was defined to each target range should contain all nodes will be "affected" by the builtin editor handling.

Therefore we cannot use these as source of truth.

No, the result are truly the browsers how the API implemented. It's useful to discuss the clearer definition. (But the expectations are not useful.)

I don't necessarily disagree with browsers behaving this way, but given that the browsers will depend on platform editing behavior, and such behavior may change in the future, I don't see how we can specify that.

I don't point the differences between platforms (at least in this issue). My point here is, multiple DOM ranges can represent almost same range in the DOM tree. E.g.,

So, there should be a rule to "normalize" target ranges around element boundaries and I'd like the spec clarify the rules. In other words, I'd like to avoid compatibility issues between browsers when they intent to return same target range.

masayuki-nakano commented 1 week ago

(And it seems that minimized ranges are same as or similar to current browser implementations. So I think that I don't suggest risky spec changes.)

johanneswilm commented 1 week ago

@masayuki-nakano If you only take the most central leave node, how would the user remove the link in a case like this?

  1. <p>Please visit my outdated website: <a href="...">h</a>[].</p>

Hitting backspace once, would create these target ranges, if I understand you right.

  1. <p>Please visit my outdated website: <a href="...">{h}</a>[].</p>

If the beforeinput event is not cancelled, the result will then look like this:

  1. <p>Please visit my outdated website: <a href="...">[]</a>.</p>

The link will be invisible, yet if the user starts typing, the typed text will show up inside the link:

4a. <p>Please visit my outdated website: <a href="...">new typed text[]</a>.</p>

If the user instead uses the arrow keys to move right and left, there will be an empty link stuck there forever, correct?

4b. <p>Please visit my outdated websi[]te: <a href="..."></a>.</p>

johanneswilm commented 1 week ago

@masayuki-nakano The two tentative tests you created and linked to here seem to say that you want getTargetRanges() to return something other than what is the affected range. See for example this comment:

// Should delete the <span> element because it becomes empty. // However, we need discussion whether the <span> element should be // contained by a range of getTargetRanges().

https://github.com/web-platform-tests/wpt/blob/868541df96894653a8037568e2137a8481e676a5/input-events/input-events-get-target-ranges-backspace.tentative.html#L74-L77

So you are saying you would want to delete the span as default action, but you don't want to include it in the target range. I don't think I understand why you want want to make a difference there. In the example in my last comment, I think it would be quite bad if an inline element is just left there without any way of being able to remove it.

However, it's harder to extend the range from performance point of view.

What performance are we talking about here? The speed of an app that receives typing is naturally limited by the human not being able to type more than at a certain speed. That's why I don't think we have the same kind of speed requirements as for example 3D games, etc. .But maybe I am just not understanding what kind of performance you are thinking of?

Additionally, even if an inline element which gives a style like <b> is deleted from the DOM, new typing text could preserve the style. In such case, including parent elements may make web apps confused. Therefore, I believe that it's the best to shrink the target range as far as possible.

I wouldn't like browser's to natively do that. But if they do, I assume the next editing operation will include the insertion of the new piece of content.

johanneswilm commented 3 days ago

I think there has been some confusion as to whether browsers should or should not reinsert inline dom nodes when typing after deletion.

I believe:

  1. It is wrong to reinsert dom inline dom nodes when typing after deleting. See https://github.com/w3c/editing/issues/468
  2. Chromium and potentially other browsers should be fixed not to do that any more. That should also make them compliant in relation to the Input Events spec.
  3. The ranges returned by getTargetRanges() mentioned above in this issue should correspond to what is actually being deleted. Fixing 2 should also remove any confusion there may still be.