1455894 - Need to teach selection about the flattened tree / frame tree instead of the light tree

Status: RESOLVED WORKSFORME

(Core :: DOM: Core & HTML, enhancement, P3)

Description

[Emilio Cobos Álvarez (:emilio)]

nsRange keeps all these offsets around in terms of the light tree.

This means that it's fundamentally flawed for Shadow DOM, and all the selection APIs, find, and such are going to be really broken when people use Shadow DOM.

We probably can afford not to fix the selection APIs to ship the first version of Shadow DOM, since they're probably not relied upon, but we should probably support basic functionality like selecting and searching in Shadow DOM.

Comment 1

[Emilio Cobos Álvarez (:emilio)]

My intuition from my not-in-depth understanding of how these APIs are consumed is that they mostly care about the flattened tree / frame tree + placeholders, and that you already can't rely on keeping ranges around before and after a flush (since script may have ran and may have fiddled with the DOM).

Is that right, Mats? If so, what do you (and the others?) think about keeping ranges in terms of the frame tree?

Seems easier than doing it in terms of the flattened tree, since a linked structure is already setup by the frame constructor, instead of having to manually compute the right links / insertion points / etc every time.

Is anyone aware of any API that would require to use ranges for undisplayed stuff?

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

Range itself isn't supposed to cross shadow boundaries. Selection may.
And Range deals with DOM (sub)trees, whether or not the tree is in shadow.

Sounds like this bug is about selection, not about range.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Also, remember that currently selection handling crossing shadow DOM is unspecified.

There are some proposals, at least https://github.com/yoichio/public-documents/blob/master/selectionapiforshadow.md

Comment 4

[Emilio Cobos Álvarez (:emilio)]

(In reply to Olli Pettay [:smaug] from comment #2)
> Range itself isn't supposed to cross shadow boundaries. Selection may.
> And Range deals with DOM (sub)trees, whether or not the tree is in shadow.
> 
> Sounds like this bug is about selection, not about range.

Hmm, I guess so, yeah. I was assuming that it would've been easier to modify nsRange than all its consumers, but that may not work for everything I guess.

Bug 1455891 is kind of its own beast from what I've seen (nsFind uses two iterators to deal with NAC in <textarea>, we'd need to change to a stack, but even that wouldn't preserve the composed tree order, which I suspect is what we want...).

Anyway, I guess we can discuss that there.

Comment 5

[Mats Palmgren (inactive)]

(In reply to Emilio Cobos Álvarez [:emilio] from comment #1)
> My intuition from my not-in-depth understanding of how these APIs are
> consumed is that they mostly care about the flattened tree / frame tree +
> placeholders, and that you already can't rely on keeping ranges around
> before and after a flush (since script may have ran and may have fiddled
> with the DOM).
> 
> Is that right, Mats? 

I'm not sure what the question is exactly, but nsRanges observes
DOM mutations and update themselves by "gravitating" towards
the nearest node, to (mostly) keep the selection intact.
See the methods after this line:
https://searchfox.org/mozilla-central/rev/f8e72641772293702e76e75620b777b730b71b72/dom/base/nsRange.cpp#516

> If so, what do you think about keeping ranges in terms of the frame tree?

We used to keep selection state in the frame tree and it was very buggy,
which is why we switched to keep the selection state in the DOM instead:
see bug 679183 and bug 619273.
I think it would be a mistake to go back to a frame solution.

> Is anyone aware of any API that would require to use ranges for undisplayed
> stuff?

I'm probably misunderstanding your question, but the DOM Range API
works perfectly fine on display:none/contents nodes...

Comment 6

[Mats Palmgren (inactive)]

(In reply to Olli Pettay [:smaug] from comment #2)
> Range itself isn't supposed to cross shadow boundaries. Selection may.
> And Range deals with DOM (sub)trees, whether or not the tree is in shadow.

Right, that's how we could solve this actually -- but splitting up
the Selection into one Range that's in the light DOM and another
Range that's in the shadow DOM.  E.g. if you select over a shadow
boundary like so: "out[erinn]er" you'd end up with:
Selection:
  Range[0]: "outer" 3, "outer" 5
  Range[1]: "inner" 0, "inner" 3

That way we shouldn't violate any assumptions (or if we do, that's
just existing bugs that we should fix).  The code to support that
already exists for the most part since that's what we do when
the user selects over 'user-select:none' islands. See
nsRange::ExcludeNonSelectableNodes. I think it would be fairly
easy to amend that to split ranges at shadow DOM boundaries.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Note that this is a longstanding problem: selection can create pretty bogus ranges (or could at one point) that span across shadow boundaries.  We had this problem with XBL too.

The real answer is that you can't use a single range to represent selection in a shadow DOM world.  Now we just have to convince other UAs to allow non-single-range selections at all.  :(

Comment 8

[Emilio Cobos Álvarez (:emilio)]

(In reply to Mats Palmgren (:mats) from comment #5)
> (In reply to Emilio Cobos Álvarez [:emilio] from comment #1)
> > My intuition from my not-in-depth understanding of how these APIs are
> > consumed is that they mostly care about the flattened tree / frame tree +
> > placeholders, and that you already can't rely on keeping ranges around
> > before and after a flush (since script may have ran and may have fiddled
> > with the DOM).
> > 
> > Is that right, Mats? 
> 
> I'm not sure what the question is exactly, but nsRanges observes
> DOM mutations and update themselves by "gravitating" towards
> the nearest node, to (mostly) keep the selection intact.
> See the methods after this line:
> https://searchfox.org/mozilla-central/rev/
> f8e72641772293702e76e75620b777b730b71b72/dom/base/nsRange.cpp#516
> 
> > If so, what do you think about keeping ranges in terms of the frame tree?
> 
> We used to keep selection state in the frame tree and it was very buggy,
> which is why we switched to keep the selection state in the DOM instead:
> see bug 679183 and bug 619273.
> I think it would be a mistake to go back to a frame solution.

Ah, makes sense, thanks!

> > Is anyone aware of any API that would require to use ranges for undisplayed
> > stuff?
> 
> I'm probably misunderstanding your question, but the DOM Range API
> works perfectly fine on display:none/contents nodes...

I was meaning that if we did it using the frame tree it wouldn't work for undisplayed stuff at all, but given the above that's not a solution.

(In reply to Mats Palmgren (:mats) from comment #6)
> (In reply to Olli Pettay [:smaug] from comment #2)
> > Range itself isn't supposed to cross shadow boundaries. Selection may.
> > And Range deals with DOM (sub)trees, whether or not the tree is in shadow.
> 
> Right, that's how we could solve this actually -- but splitting up
> the Selection into one Range that's in the light DOM and another
> Range that's in the shadow DOM.  E.g. if you select over a shadow
> boundary like so: "out[erinn]er" you'd end up with:
> Selection:
>   Range[0]: "outer" 3, "outer" 5
>   Range[1]: "inner" 0, "inner" 3

I guess we need arbitrary selections, not just two, since you can have arbitrarily nested shadow trees... But yeah, that sounds fine I think.

Comment 9

[Mats Palmgren (inactive)]

(In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #7)
> The real answer is that you can't use a single range to represent selection
> in a shadow DOM world.  Now we just have to convince other UAs to allow
> non-single-range selections at all.  :(

Perhaps the best way forward is to implement a prototype using multi-range
Selection?  To demonstrate that it's a viable, perhaps better, alternative.

Fwiw, TabAtkins was positive to multi-range Selection last this came up on
www-style (in the context of selection in Grid layout etc, which doesn't
really behave sanely without them).  Now, with their LayoutNG effort,
might be a good time to win them over to this model...

Comment 11

[Emilio Cobos Álvarez (:emilio)]

Looks like WebKit fixed this (https://bugs.webkit.org/show_bug.cgi?id=151380), so we're the only engine that supports Shadow DOM and doesn't support this properly...

Comment 12

[Olli Pettay [:smaug][[e-mail]]]

Hmm, in which way did they fix it. I guess not implementing the proposed selection handling yet, but something random.

Comment 13

[Firefox Bug Husbandry Bot]

See bug 1547409. Migrating webcompat priority whiteboard tags to project flags.

Comment 14

[Thomas Wisniewski [:twisniewski]]

For testing reference, the following web platform tests exist which seem to test selection behavior in shadow roots:

• html/editing/editing-0/contenteditable/contentEditable-slotted-inherit.html
• shadow-dom/untriaged/shadow-trees/upper-boundary-encapsulation/test-009.html
• shadow-dom/untriaged/user-interaction/ranges-and-selections/test-001.html
• shadow-dom/untriaged/user-interaction/ranges-and-selections/test-002.html
• shadow-dom/untriaged/elements-and-dom-objects/shadowroot-object/shadowroot-methods/test-004.html

However, since Firefox nightly and Safari 87 preview don't differ in whether they pass or fail those tests, I'm guessing none of them cover this bug. But if WebKit did indeed fix their implementation in the referenced commit above, then we can likely convert the tests it added to their tree as the basis for some new testdriver-based web platform tests here.

Comment 15

[HansHuckebein]

More and more websites use this feature and as far as I see, Firefox is the only browser, which is too stupid to correctly select text on these sites. This is very poor.

Comment 16

[HansHuckebein]

This bug has been known for 5 years now. The html feature that triggers it is being used more and more.

When are you going to get this shit fixed?

Comment 17

[Mike Hoye [:mhoye]]

This is a polite reminder that Bugzilla is our professional working environment as well as our issue tracker. I encourage you to review our participation and etiquette guidelines; comments that help move issues towards a resolution are always welcome. Comments that add nothing more than demands that a resolution occur, however, are not.

Comment 18

[Calin Tanase]

This seems fixed based on the issue tested from https://github.com/webcompat/web-bugs/issues/70859 . It no longer reproduces on Firefox Nightly, only on Firefox Release.

Emilio, should we close this report as fixed?

Environment:
Operating system: Windows 10
Browsers tested: Firefox Nightly 128.0a1 (2024-05-28) / Firefox Release 126 / Chrome 125.0.6422.113

Comment 19

[Emilio Cobos Álvarez (:emilio)]

Yeah Sean has done / is doing a lot of work on shadow-crossing selection.