-
Notifications
You must be signed in to change notification settings - Fork 31
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
Need spec changes to Range and StaticRange to support nodes in different tree? #169
Comments
The other thing is whether we want to change the comparison somehow to account for slots, even if the range doesn't cross shadow boundaries per se. |
An example of the node comparison issue that Emilio pointed
The above console.log prints true, saying FirstElement is in the range, however it's not visually selected. |
I see a few ways to solve this.
The same logic should also be applied for Range setEnd/setStart. |
I very much like this option. The whole point of the overall
Same here - let's just make this work correctly. I went into this particular case in some detail in my explainer: |
I'm not sure that changing the definition of "valid" would be correct. We'd have to look at the callers. We might well need valid and shadow-including valid as separate concepts. It's also not clear to me we can just change the public API of |
The only web API that I can find that returns a StaticRange is https://w3c.github.io/input-events/#dom-inputevent-gettargetranges. Do you know of others?
I think the proposal is to only change the public API of Perhaps your comment was about this part of the comment?
But that just refers to this behavior: https://github.com/mfreed7/shadow-dom-selection?tab=readme-ov-file#changes-to-existing-selection-apis |
As per https://dontcallmedom.github.io/webdex/v.html#valid%40%40StaticRange%40dfn the valid concept is used in https://drafts.csswg.org/css-highlight-api-1/ which is also what we added it for. I haven't done detailed review myself though. |
Ahh ok, thanks for the pointer. So the highlight API takes either
From that paragraph (and the explicit usage of shadow including root specifically), it sounds like the intention was actually to support ranges that cross shadow boundaries, no? I suppose there's a risk of compat problems if we change the definition of If there are other references to StaticRange, it'd be good to look at each of them to analyze compat. |
@johanneswilm could you please also Agenda+ this issue for TPAC? Thank you |
There are multiple spec changes that we should consider to support a Range with nodes in different trees. Here is a summary of the above conversations: 1. Output of getComposedRangesCurrently, the function returns a list of StaticRange. However, this violates the spec rule for what is a valid StaticRange:
Potential changes so we don't violate the spec: 2. When to use Flat tree traversal to compare positions
In the above steps, the before/after/equal position definitions are for within the same tree.
How do we update the specs to allow nodes in different trees? Maybe define new concepts, maybe Should we always use FlatTreeTraversal when storing this composed range? Or, should it only be stored when we have nodes in different trees and are crossing shadow boundaries? First option makes the most sense because of slotted contents. Those nodes should be compared using Flat tree, even if in the same tree. 3. Selection::containsNode() mismatchCurrently, this is returning weird results for slotted cases since all comparisons are using DOM tree traversal. We should probably keep that behavior as is. Bonus: Should we consider adding other functions such as toString()? |
Summary of conversation at TPAC 1. Output of getComposedRangesRESOLVED to keep the definition as is and still return a StaticRange. The definition of valid was added for the Highlights API and is not web exposed. Maybe that definition should be changed to "highlights valid" or be removed. 2. When to use Flat tree traversal to compare positionsCurrent spec definition for before/after is meant for composed tree (DOM tree element traversal within the document). We might want to consider adding new definition in HTML such as "shadow-inclusive before", "shadow-inclusive after", etc. The current getComposedRanges() expects to use the Composed tree traversal. A lot of conversation was had about. I created the new issue #336 to continue the conversation and better evaluate the needs. 3. Selection::containsNode() mismatchThere is a lot of opportunities in the Selection API to make sense out of toString(), isPointInRange(), etc. Browsers behave very differently and all implemented seemed interested to improve the interoperability. There is an existing issue about how to serialize across trees: #163 |
From TPAC 2024 minutes:
|
It's unclear to me about how we want to express allowing nodes across the shadow boundary for
range
in the spec.The first example is StaticRange has a definition for valid, which specifies this StaticRange is invalid if nodes are in different tree. In the same time,
getComposedRanges()
would return invalid StaticRanges. Is this expected?The second example is
Range::SetStart
andRange::SetEnd
would collapse nodes to one point if they are in different trees. Shouldn't we also loose this requirement since we are supporting selection across the boundary?The text was updated successfully, but these errors were encountered: