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

Define "affected ranges" clearer at InputEvent.getTargetRanges() definition #112

Open
masayuki-nakano opened this issue Jul 28, 2020 · 13 comments

Comments

@masayuki-nakano
Copy link

Currently, getTargetRanges() is defined as:

getTargetRanges() returns an arrays StaticRanges that will be affected by the event if it is not cancelled.
https://w3c.github.io/input-events/#dom-inputevent-gettargetranges

InputEvent.getTargetRanges(): returns an arrays of StaticRanges which will be affected by the change to the DOM if it is not canceled.
https://w3c.github.io/input-events/#event-type-beforeinput

Currently, I'm implementing computation of "affected ranges" for shipping beforeinput event of Firefox. However, this definition is really unclear.

For example, when Backspaceing in <p>abc</p><p>[]def</p>, then, the paragraphs will be joined so that ("abc", 3) - ("def", 0) must be expected by web apps even though new empty first or second <p> element is removed (i.e., affected).

Another example: when there is invisible white-spaces at start of second paragraph and/or end of first paragraph in the previous example, i.e., <p>abc </p><p> []def</p>, browsers removes the invisible white-spaces too, i.e., becomes <p>abcdef</p>. So, in this case, getTargetRanges() should include the invisible white-spaces, but at least Blink does not include the invisible white-spaces.

On the other hand, if there is an invisible <br> element in the first paragraph, i.e., in the case of <p>abc<br></p><p>[]def</p>, Blink returns (p, 1) - (p, 0). I.e., including the invisible <br> element.

So, at least the first example, "affect range" may not be good information which web apps want to know. However, at the latter 2 cases, invisible things which will be removed should be contained by the result of getTargetRanges().

moz-wptsync-bot pushed a commit to web-platform-tests/wpt that referenced this issue Aug 4, 2020
There are no automated tests for `InputEvent.getTargetRanges()` because it
is set only when `beforeinput` event, but it's defined as not dispatched by
`document.execCommand`.

However, we can synthesize `beforeinput` event with test driver.

On the other hand, the definition in Input Events spec is not clear.
Therefore, most of the tests won't be passed on any browsers for now.
There are some spec issues which I filed:
* w3c/input-events#112
* w3c/input-events#113
* w3c/input-events#114

These new test must be useful when browser vendors discuss the issues.

Differential Revision: https://phabricator.services.mozilla.com/D85527

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1655393
gecko-commit: 12f21ad909371384939bb38d65cf1dbc797b5bf9
gecko-integration-branch: autoland
gecko-reviewers: smaug
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Aug 4, 2020
There are no automated tests for `InputEvent.getTargetRanges()` because it
is set only when `beforeinput` event, but it's defined as not dispatched by
`document.execCommand`.

However, we can synthesize `beforeinput` event with test driver.

On the other hand, the definition in Input Events spec is not clear.
Therefore, most of the tests won't be passed on any browsers for now.
There are some spec issues which I filed:
* w3c/input-events#112
* w3c/input-events#113
* w3c/input-events#114

These new test must be useful when browser vendors discuss the issues.

Differential Revision: https://phabricator.services.mozilla.com/D85527
moz-wptsync-bot pushed a commit to web-platform-tests/wpt that referenced this issue Aug 4, 2020
There are no automated tests for `InputEvent.getTargetRanges()` because it
is set only when `beforeinput` event, but it's defined as not dispatched by
`document.execCommand`.

However, we can synthesize `beforeinput` event with test driver.

On the other hand, the definition in Input Events spec is not clear.
Therefore, most of the tests won't be passed on any browsers for now.
There are some spec issues which I filed:
* w3c/input-events#112
* w3c/input-events#113
* w3c/input-events#114

These new test must be useful when browser vendors discuss the issues.

Differential Revision: https://phabricator.services.mozilla.com/D85527

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1655393
gecko-commit: 12f21ad909371384939bb38d65cf1dbc797b5bf9
gecko-integration-branch: autoland
gecko-reviewers: smaug
xeonchen pushed a commit to xeonchen/gecko that referenced this issue Aug 4, 2020
There are no automated tests for `InputEvent.getTargetRanges()` because it
is set only when `beforeinput` event, but it's defined as not dispatched by
`document.execCommand`.

However, we can synthesize `beforeinput` event with test driver.

On the other hand, the definition in Input Events spec is not clear.
Therefore, most of the tests won't be passed on any browsers for now.
There are some spec issues which I filed:
* w3c/input-events#112
* w3c/input-events#113
* w3c/input-events#114

These new test must be useful when browser vendors discuss the issues.

Differential Revision: https://phabricator.services.mozilla.com/D85527
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Aug 6, 2020
There are no automated tests for `InputEvent.getTargetRanges()` because it
is set only when `beforeinput` event, but it's defined as not dispatched by
`document.execCommand`.

However, we can synthesize `beforeinput` event with test driver.

On the other hand, the definition in Input Events spec is not clear.
Therefore, most of the tests won't be passed on any browsers for now.
There are some spec issues which I filed:
* w3c/input-events#112
* w3c/input-events#113
* w3c/input-events#114

These new test must be useful when browser vendors discuss the issues.

Differential Revision: https://phabricator.services.mozilla.com/D85527

UltraBlame original commit: 12f21ad909371384939bb38d65cf1dbc797b5bf9
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Aug 6, 2020
There are no automated tests for `InputEvent.getTargetRanges()` because it
is set only when `beforeinput` event, but it's defined as not dispatched by
`document.execCommand`.

However, we can synthesize `beforeinput` event with test driver.

On the other hand, the definition in Input Events spec is not clear.
Therefore, most of the tests won't be passed on any browsers for now.
There are some spec issues which I filed:
* w3c/input-events#112
* w3c/input-events#113
* w3c/input-events#114

These new test must be useful when browser vendors discuss the issues.

Differential Revision: https://phabricator.services.mozilla.com/D85527

UltraBlame original commit: 12f21ad909371384939bb38d65cf1dbc797b5bf9
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Aug 6, 2020
There are no automated tests for `InputEvent.getTargetRanges()` because it
is set only when `beforeinput` event, but it's defined as not dispatched by
`document.execCommand`.

However, we can synthesize `beforeinput` event with test driver.

On the other hand, the definition in Input Events spec is not clear.
Therefore, most of the tests won't be passed on any browsers for now.
There are some spec issues which I filed:
* w3c/input-events#112
* w3c/input-events#113
* w3c/input-events#114

These new test must be useful when browser vendors discuss the issues.

Differential Revision: https://phabricator.services.mozilla.com/D85527

UltraBlame original commit: 12f21ad909371384939bb38d65cf1dbc797b5bf9
@masayuki-nakano
Copy link
Author

masayuki-nakano commented Sep 15, 2020

Gecko compute "affected ranges" as the following rules for shipping in Nightly channel:

If Selection ranges are collapsed:

  • If collapsed around a character in a text node, selects a character which means that it may select a surrogate pair if starting from a high-surrogate at forward deleting or a low surrogate at backward deletion
  • If collapsed around an atomic content such as <input>, <img>, visible <br>, select it (i.e., at it in its parent and ends with next offset in same parent)
  • If collapsed at inner of a block boundary, "affected range" contains only the boundary. For example, if pressing Backspace at <div>abc<p>[]def</p></div>, the range is <div>abc[<p>]def</p></div>. I.e., if the range starts and/or ends with a text node, its end and/or start becomes the boundaries of the range.
  • If collapsed at outer of a block boundary, "affected range" contains only the boundary. For example, if pressing Delete at <div>abc[]<p>def</p></div>, the range is <div>abc[<p>]def</p></div>. I.e., if the range starts and/or ends with a text node, its end and/or start becomes the boundaries of the range.
  • If collapse around an atomic content, select only the node, e.g., <p>abc<img>[]</p> and press Backspace, the range should be {P, 1} - {P, 2}.

However, if the range surrounded by invisible white-spaces, and they will be deleted by the action, they should be included into the target range. E.g., if <p>abc<hr> []def</p>, for keeping the white-space invisible, the range should be {p, 1} - {" def", 1}.

If the range is not collapsed, basically, it should be as-is, however, deleting the range causes deleting surrounding invisible white-spaces, the deleting invisible white-spaces should be included in the range. For example, <p>abc [</p><p>] def</p>, the range should be <p>abc[ <p><p> ]def</p>.

On the other hand, if non-collapsed range contains only one atomic content, but the range boundaries are in text nodes, e.g., <p>abc[<img>]def</p>, the target range should be as-is because trying to shrink the range is expensive for the browsers. For providing better UX for the users, browsers shouldn't try to shrink non-collapsed range(s).

Finally, I have no idea about this case: If selection range across table cells, like <table><tr><td>ab[c</td><td>d]ef</td></tr></table>, browsers won't change the table structure. Instead, they delete the content in the range. I.e., it becomes <table><tr><td>ab</td><td>ef</td></tr></table>. In this case, returning {"abc", 2} - {"def", 1} may not useful for web apps. Should browsers return {"abc", 2} - {"abc", 3} and {"def", 0} - {"def", 1} instead?

@masayuki-nakano
Copy link
Author

masayuki-nakano commented Sep 15, 2020

And there is an issue if deleting block boundaries:

<div><b>abc</b><p><b>def</b></p></div>

If the <b> elements are joined, I think that the affected range should be <div><b>abc[</b><p><b>]def</b></p></div>, and if the <b> elements are not joined, I think that the affected range should be <div><b>abc</b>{<p>}<b>def</b></p></div>. That means that the "affected range" includes element boundaries which will be joined. However, if we'd take this rule, there are complicated cases. For example, <div><i><b>abc[</b></i><p><b>]def</b></p></div>, and deleting it gets <div><b><i>abc</i>def</b></div>, then, it's impossible to represent a range including only the element boundaries deleted. So, I think that "affected ranges" shouldn't take care of inline element boundaries which will be joined for making browser faster.

@annevk
Copy link
Member

annevk commented Nov 16, 2020

@RByers @whsieh could you work with @masayuki-nakano to improve this definition and interoperability between implementations? (Or find someone within your organization with the relevant expertise.)

@masayuki-nakano
Copy link
Author

I added a lot of tentative tests to WPT, but it's not enough (editor has too many features!!)
https://wpt.fyi/results/input-events?label=master&label=experimental&aligned&q=input-events-get-target-ranges-

@masayuki-nakano
Copy link
Author

It seems that when a target range boundary is at start of a block, Blink adjusts it as 0 of the block, but when a target boundary is at end of a block whose last content is a text node, Blink adjusts it as end of the text node, rather than end of the block. This inconsistency does not make sense. And I guess that pointing in a leaf node is more convenient for web apps. E.g., scanning ancestor block is easier than scanning first editable leaf.

@johanneswilm
Copy link
Contributor

johanneswilm commented Jan 26, 2021

Hey,
to clarify: When we wrote the spec in the editing taskforce, we did not want to specify how backspace and delete behave or how the caret is to move. It's not that it would be bad to do so, but doing that seemed like a gigantic task with many complications and exceptions and the browser makers who participated were not willing to do this at the time. Can we differentiate between the following issues here?

  • A language issue of the term "affected range" not expressing clearly whether or not DOM nodes at a higher level should be included. So for example if you press backspace in this case <p>abc</p><p>[]def</p> it could be argued that the entire second paragraph-element is "affected" as there will only be one paragraph element after the operation. It seems like all implementers have understood this correctly so far. If there is a better term, we could adjust the language to make it clearer that the entire second paragraph should not be included if we have a way of expressing that. The term currently used is the best we could come up with in the editing taskforce at the time.

  • An issue of Blink returning a target range that differs from the actually affected range by not including certain types of invisible white space that really is affected. This is something that should be fixable in the blink engine without changing the spec, correct?

  • There is some inconsistency in how Blink describes target ranges, sometimes referring to position zero in a text node and at other times referring to position zero in the parent node. I am not sure this is a problem. Is it? Given that the text nodes end up being merged, the more correct behavior would b for the static range to specify a place within the text node rather than the parent node.

  • Browsers at times do some DOM cleanup (normalization, etc.). This is generally problematic for JavaScript editors and several browsers have cut down on this over the past 5-8 years. If this happens after a dom modification that causes a beforeinput event it can cause even more problems. Browsers should try to cut down on this in order to be friendly to JS editors, but also this is outside of the scope of this spec unless we create a special inputType for these kinds of changes.

  • Different browsers have different backspace/delete behavior. This would be good to get standardized eventually so that JS apps can predict what will happen, but that would be outside of the scope of the Input Events spec.

Is that correctly understood?

@johanneswilm
Copy link
Contributor

johanneswilm commented Jan 26, 2021

For example, <div><i><b>abc[</b></i><p><b>]def</b></p></div>, and deleting it gets <div><b><i>abc</i>def</b></div>, then, it's impossible to represent a range including only the element boundaries deleted. So, I think that "affected ranges" shouldn't take care of inline element boundaries which will be joined for making browser faster.

I am not sure how that selection could be set like that through human input, but putting it either here <div><i><b>abc[]</b></i><p><b>def</b></p></div> or here: <div><i><b>abc</b></i><p><b>[]def</b></p></div> and then hitting delete/backspace respectively, I get <div><i><b>abc</b></i><b>def</b></div>. So basically the <p> is gone, but the <b>s are not joined. The same happens on Firefox and Chrome.

This looks like one of those examples where browser in the past might have moved the dom nodes around (merging the <b>s), not realizing that this actually creates all kinds of problems for JS editors. This current behavior seems much better.

@johanneswilm
Copy link
Contributor

On the other hand, if non-collapsed range contains only one atomic content, but the range boundaries are in text nodes, e.g., <p>abc[<img>]def</p>, the target range should be as-is because trying to shrink the range is expensive for the browsers. For providing better UX for the users, browsers shouldn't try to shrink non-collapsed range(s).

I think the cost argument should not be important here. The target ranges only need to be calculated if the getTargetRanges() function is called and if it just returns the same as the selection if the selection is not collapsed even when that will not return the real target range, then what is the point of calling the function at all? Can the JS author not just take the selection direct? If we do think it is important whether or not the target range starts in a text node or in the parent node, then the getTargetRanges() function should be the most correct ranges possible. So in the above example that would be {"abc", 3} - {"def", 0}.

We could also say that we don't care whether the range starts in the parent node or the text node because we know that adjacent text nodes as a result of browser native insertion/deletion will always be merged.

@masayuki-nakano
Copy link
Author

@johanneswilm:

to clarify: When we wrote the spec in the editing taskforce, we did not want to specify how backspace and delete behave or how the caret is to move.

I agree with that. I think that it should be based on the platform manner. But I don't like the current definition because it's almost same as "undefined".

* A language issue of the term "affected range" not expressing clearly whether or not DOM nodes at a higher level should be included. So for example if you press backspace in this case `<p>abc</p><p>[]def</p>` it could be argued that the entire second paragraph-element is "affected" as there will only be one paragraph element after the operation. It seems like all implementers have understood this correctly so far. If there is a better term, we could adjust the language to make it clearer that **the entire second paragraph should not be included** if we have a way of expressing that. The term currently used is the best we could come up with in the editing taskforce at the time.

If we don't take care of inline elements, it might be explained as the range should start from a DOM point whose data is deleted and end by a DOM point whose data will keep alive even in different node. (But as you say, it's really difficult thing and I understand why you haven't tackled it.) So, affected range is defined as a range between data in text nodes, it could be defined simpler (except when deleting an element like <img>, <br>).

* An issue of Blink returning a target range that differs from the actually affected range by not including certain types of invisible white space that really is affected. This is something that should be fixable in the blink engine without changing the spec, correct?

I think so. But ideally spec defines it with some text which make web developers understand as such invisible white-spaces should be included in the "affected range".

* There is some inconsistency in how Blink describes target ranges, sometimes referring to position zero in a text node and at other times referring to position zero in the parent node. I am not sure this is a problem. Is it? Given that the text nodes end up being merged, the more correct behavior would b for the static range to specify a place within the text node rather than the parent node.

I don't have any concrete idea of web developers. So, I don't know whether the difference of container of target range boundary becomes a trouble or not. But as an implementer, the different is a painful point for web-compat. I just want more detail in the spec for explaining why such incompatible thing may occur with pointing a spec's section.

Ideally, inline element boundaries which will be joined with another element should be included in the "affected ranges", but I think that this is probably impossible in some cases. E.g., backspacing at <b><i>foo</i></b><i>[]bar</i> may become <i><b>fo</b>bar</i>. In this case, <i> element is merged, but <b> element is not, so, it cannot represent only <i> element boundary with a range. So, it might be better if affected ranges do not mean including element boundaries which are joined.

* Browsers at times do some DOM cleanup (normalization, etc.). This is generally problematic for JavaScript editors and several browsers have cut down on this over the past 5-8 years. If this happens after a dom modification that causes a beforeinput event it can cause even more problems. Browsers should try to cut down on this in order to be friendly to JS editors, but also this is outside of the scope of this spec unless we create a special inputType for these kinds of changes.

Yeah, it's not a realistic case for every change has a specific inputType value. It's sad though.

* Different browsers have different backspace/delete behavior. This would be good to get standardized eventually so that JS apps can predict what will happen, but that would be outside of the scope of the Input Events spec.

Yes. My point is, it should be defined as far as possible how to represented as the affected ranges for coming DOM mutation.

Is that correctly understood?

I think so.

For example, <div><i><b>abc[</b></i><p><b>]def</b></p></div>, and deleting it gets <div><b><i>abc</i>def</b></div>, then, it's impossible to represent a range including only the element boundaries deleted. So, I think that "affected ranges" shouldn't take care of inline element boundaries which will be joined for making browser faster.

I am not sure how that selection could be set like that through human input, but putting it either here <div><i><b>abc[]</b></i><p><b>def</b></p></div> or here: <div><i><b>abc</b></i><p><b>[]def</b></p></div> and then hitting delete/backspace respectively, I get <div><i><b>abc</b></i><b>def</b></div>. So basically the <p> is gone, but the <b>s are not joined. The same happens on Firefox and Chrome.

This looks like one of those examples where browser in the past might have moved the dom nodes around (merging the <b>s), not realizing that this actually creates all kinds of problems for JS editors. This current behavior seems much better.

Thank you for the testing. But IIRC, in some cases, inline elements may be joined. Although I don't have examples.

On the other hand, if non-collapsed range contains only one atomic content, but the range boundaries are in text nodes, e.g., <p>abc[<img>]def</p>, the target range should be as-is because trying to shrink the range is expensive for the browsers. For providing better UX for the users, browsers shouldn't try to shrink non-collapsed range(s).

I think the cost argument should not be important here. The target ranges only need to be calculated if the getTargetRanges() function is called and if it just returns the same as the selection if the selection is not collapsed even when that will not return the real target range, then what is the point of calling the function at all?

Oh, the definition sounds reasonable to me for explaining "affected ranges" except when expanding the range to delete surrounding invisible white-spaces.

Can the JS author not just take the selection direct? If we do think it is important whether or not the target range starts in a text node or in the parent node, then the getTargetRanges() function should be the most correct ranges possible. So in the above example that would be {"abc", 3} - {"def", 0}.

Yeah, it might be reasonable if browsers can ignore the runtime cost to walk the tree.

We could also say that we don't care whether the range starts in the parent node or the text node because we know that adjacent text nodes as a result of browser native insertion/deletion will always be merged.

Yeah.

@johanneswilm
Copy link
Contributor

ideally spec defines it with some text which make web developers understand as such invisible white-spaces should be included in the "affected range".

OK just to understand why a clarification is needed here: For example right now it will return something like {"abc ", 4}, when the start or end that is really being changed is {"abc ", 3}. I wonder a bit how one could explain the current output (short of saying it is a bug in blink). I mean it's not that the space was just ignored. It was counted - that's why it's 4 rather than 3. Are we to understand this as that there is one first operation of merging two text nodes which we get the target ranges for and then there is a second cleanup operation which removes extra spaces and this second operation is not accounted for when returning the target ranges?

For example, <div><i><b>abc[</b></i><p><b>]def</b></p></div>, and deleting it gets <div><b><i>abc</i>def</b></div>, then, it's impossible to represent a range including only the element boundaries deleted. So, I think that "affected ranges" shouldn't take care of inline element boundaries which will be joined for making browser faster.

I am not sure how that selection could be set like that through human input, but putting it either here <div><i><b>abc[]</b></i><p><b>def</b></p></div> or here: <div><i><b>abc</b></i><p><b>[]def</b></p></div> and then hitting delete/backspace respectively, I get <div><i><b>abc</b></i><b>def</b></div>. So basically the <p> is gone, but the <b>s are not joined. The same happens on Firefox and Chrome.

This looks like one of those examples where browser in the past might have moved the dom nodes around (merging the <b>s), not realizing that this actually creates all kinds of problems for JS editors. This current behavior seems much better.

Thank you for the testing. But IIRC, in some cases, inline elements may be joined. Although I don't have examples.

to join two inline nodes when they are next to each other is likely fine in many situations. It's the restacking that can cause problems though as the browser may not always understand the significance of everything. Take for example something like this:

<div><p><i><b>abc<span class="citation" data-id="14" contenteditable="false">(Einstein 1998: p. 45)</span>[]</b></i></p><p><b><span class="reference" data-id="a45">def</span></b></p></div>

If I hit Delete, this will move the contents of the second paragraph into the first paragraph and remove the second paragraph. But if it were to additionally move the inline dom nodes around without destroying the content? Now knowing what the meaning of the additional span is, it would likely come up with something like:

<div><p><b><i>abc(Einstein 1998: p. 45)[]</i>def</b></p></div>

or

<div><p><b><i>abc</i><span class="citation reference"><i>(Einstein 1998: p. 45)[]</i>def</span></b></p></div>

This is one of the reasons why JS editors have asked browsers to please not try to "be smart" about it and just do the minimum required changes. And as far as I am aware, browsers are much better about this now than they used to be.

Can the JS author not just take the selection direct? If we do think it is important whether or not the target range starts in a text node or in the parent node, then the getTargetRanges() function should be the most correct ranges possible. So in the above example that would be {"abc", 3} - {"def", 0}.

Yeah, it might be reasonable if browsers can ignore the runtime cost to walk the tree.

Ok, but this cost is only there in case the user runs the getTargetRanges() function, right? And it's a cost that then still would be there if the JS editor needs to do the adjustment of the target range in JavaScript, or did I misunderstand that?

If we don't take care of inline elements, it might be explained as the range should start from a DOM point whose data is deleted and end by a DOM point whose data will keep alive even in different node. (But as you say, it's really difficult thing and I understand why you haven't tackled it.)

Ok, how about we ask for language suggestions on that from the participants of the February call. I think there is no actual disagreement about what we are trying to express at this stage. It's just a matter of putting it into the right words.

I don't have any concrete idea of web developers. So, I don't know whether the difference of container of target range boundary becomes a trouble or not. But as an implementer, the different is a painful point for web-compat. I just want more detail in the spec for explaining why such incompatible thing may occur with pointing a spec's section.

Ok, I'll try to obtain some feedback on just that point from JS editor developers. If they don't care, then I think we should be free to define this in a very simple way, for example by always preferring a reference into the parent node rather than the text node if it is on a boundary. If they do care, then we probably should take their views into account as well even if that means ending up with a slightly more complex definition.

@masayuki-nakano
Copy link
Author

masayuki-nakano commented Feb 1, 2021

@johanneswilm

I mean it's not that the space was just ignored. It was counted - that's why it's 4 rather than 3. Are we to understand this as that there is one first operation of merging two text nodes which we get the target ranges for and then there is a second cleanup operation which removes extra spaces and this second operation is not accounted for when returning the target ranges?

In my understanding, getTargetRanges() should return ranges which will be completely removed/replaced by the following default action(s). So, even if the default action is performed separately in browser engine, all of them should be included into the range.

This is one of the reasons why JS editors have asked browsers to please not try to "be smart" about it and just do the minimum required changes. And as far as I am aware, browsers are much better about this now than they used to be.

Thank you for the interesting example. And you're right. If inline elements have different style and/or editable state, they should never be joined. But as a web browser engine developer, I'd like to join inline elements if it safe to do it because it saves footprint and cost of walking DOM tree at following operations. Perhaps, when doing it, browsers should be more careful.

Ok, but this cost is only there in case the user runs the getTargetRanges() function, right? And it's a cost that then still would be there if the JS editor needs to do the adjustment of the target range in JavaScript, or did I misunderstand that?

No, as far as I've tested, getTargetRanges should be fixed the its result when it's dispatched.
The tentative WPT: https://github.com/web-platform-tests/wpt/blob/master/input-events/input-events-get-target-ranges-during-and-after-dispatch.tentative.html
Result: https://wpt.fyi/results/input-events/input-events-get-target-ranges-during-and-after-dispatch.tentative.html?label=experimental&label=master&aligned

The 2nd test in it is important. And this behavior is consistent with untrusted beforeinput event since getTargetRanges can be set only by the dictionary at creating the instance.
https://w3c.github.io/input-events/#interface-InputEvent

So, when beforeinput event is dispatched, the result needs to be computed even if it won't be used.

FYI: Firefox 87 will be the first version to support beforeinput event in the release channel. The initial behavior is similar to current Chrome. Once spec and Chrome update the behavior for partially supporting cancelable IME composition, we'll follow the behavior too.
https://groups.google.com/g/mozilla.dev.platform/c/C_92-abaiuw

@masayuki-nakano
Copy link
Author

Ah, but according to the test result, Chrome computes getTargetRanges at first call and caches it for 2nd or later calls. But if so, the result depends on when it's called. It may make really hard to debug if third party code uses getTargetRanges.

@Reinmar
Copy link

Reinmar commented Feb 2, 2021

I've been asked to give our opinion here. I'll try to reply based on the two scenarios I've been given. I hope it helps.

Currently Blink at times returns the parent node instead if the range ends at the beginning or end of the text node. So for this: <p>abc[]<p><p>def</p> If you hit Delete, blink will return a target range of {"abc", 3} - {p1, 0} (from textnode to

rather than from textnode to textnode).

Such things are completely transparent for us. For CKEditor's data model this is one thing anyway – a position before a text node and at position 0 in that text node.

In my experience, such things require normalization in our code because there are many selection/position-related browser APIs and one of the browsers will always return something slightly different. Perfect alignment between browsers would be nice, but it's not on the top of my priority list.

Blink ignores whitespace when returning target ranges. So for this: <p>abc <p><p>[]def</p>. If you hit Backspace, it will delete the space beyond the "c", so you end up with this:
<p>abc[]def</p>. Yet the target ranges will I've you a start of {"abc ", 4} rather than {"abc", 3}.

Interestingly, this does not apply to our case. We'll never render a normal space at the end of a block. If there's a trailing space in our data model, we're going to render it as nbsp. Otherwise, the browser will not be able to show a caret after that space. The space is removed from the visual representation – it means that normalization is applied at this stage anyway.

Naturally, when pressing Backspace, the browser must do more than just merge two paragraphs. It needs to remove that space that wasn't visible anyway, but would now be visible between "abc" and "def".

Do we need that space inside the targetRanges? No. We do not allow this particular situation to happen.

Do I think it should be inside targetRanges? Yes. I think it would make beforeInput easier to use it if it was there. Even if in this case our platform secures us from such situations, there may be other situations (e.g. in IME) where more precise directions regarding what needs to be removed/replaced will be necessary.

So, to me – targetRanges in this type of beforeInput means all the leaf nodes (and parts of them) that need to be removed. So, the 2nd paragraph does not need to be inside one of the target ranges as it's not a leaf node. But the space is part of the text node which is a leaf node.

@johanneswilm johanneswilm added the Agenda+ Queue this item for discussion at the next WG meeting label Feb 11, 2021
@johanneswilm johanneswilm removed the Agenda+ Queue this item for discussion at the next WG meeting label Aug 13, 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
Development

No branches or pull requests

4 participants