[css-highlight-api] Specifying behavior for Highlights involving multiple documents

[dandclark]

The Highlight API spec doesn't really consider cases where Highlights exist in multiple documents. The use of same-domain iframes bring up some interesting cases.

Case 1: A Highlight is registered in multiple registries.

let iframe = document.createElement("iframe");
document.body.appendChild(iframe);
let r = new Range();
r.setStart(document.body, 0);
r.setStart(document.body, 1);
let h1 = new Highlight(r);
CSS.highlights.set("foo", h1);
iframe.contentWindow.CSS.highlights.set("foo", h1);

This case breaks some assumptions of the current Chromium implementation, where a Highlight only has a pointer back to a single HighlightRegistry instance. The language of the spec also seems to assume that a highlight registry only contains highlights from a single document, but there is nothing explicitly blocking the scenario in the code above.

https://drafts.csswg.org/css-highlight-api-1/#registration:
"The highlight registry is accessed via the highlights attribute of the CSS namespace, and represents all the custom highlights registered for the current global object’s associated Document."

It's not completely clear what should happen here. Should it be possible for a Highlight to be added to multiple registries? If so, should changes to its contents trigger repaints for the documents associated with both registries?

Case 2: A Highlight has ranges from multiple documents

let iframe = document.createElement("iframe");
document.body.appendChild(iframe);
let r1 = new Range();
r1.setStart(document.body, 0);
r1.setStart(document.body, 1);
let r2 = new Range();
r2.setStart(iframe.contentDocument.body, 0);
r2.setStart(iframe.contentDocument.body, 1);
let h1 = new Highlight(r1, r2);
CSS.highlights.set("foo", h1);

The language in https://drafts.csswg.org/css-highlight-api-1/#range-invalidation says that the UA must ignore ranges not in a document tree, but doesn't consider the case where ranges are in a different document tree from the one associated with the given highlight registry.

Again it's not clear what should happen here. Should the UA ignore ranges that are in a document tree different from the one associated with the highlight registry containing the Highlight?

Solutions

An incremental fix for these corner cases would be to specify the following:

1. A Highlight can be added to multiple highlight registries. In Chromium, this would mean that Highlight would store a list of pointers back to highlight registries, instead of a single pointer.
2. When computing how to render a document, if a range in the highlight registry associated with that document is not in that document tree, ignore that range.

A result of doing things this way is that if a Highlight has ranges that are in different document trees, both ranges will be painted if and only if the Highlight was added to the highlight registry for both ranges.

A more adventurous idea that simplifies this somewhat is to consider whether Highlight can be eliminated altogether, such that ranges are added directly to the highlight registry. After #6198, Highlight is no longer needed to store the name. That leaves only priority. Could priority be specified some other way? One idea is to have a method to do this directly on the registry, e.g. CSS.highlights.setPriority("name", 42). Or, maybe the priority could be provided in CSS:

::highlight(name) {
  background-color: orange;
  priority: 42;
}

I don't immediately see a reason not to specify the priority through the cascade like this, so it might be worth considering as a way to eliminate Highlight and simplify the API, thus making it easier to handle the cases described above involving multiple document trees.

[dandclark]

Pinging some folks who might have thoughts on these cross-document cases: @frivoal, @fantasai, @hober.

I wonder how Webkit's implementation handles this?

[sanketj]

RE priority: See the full discussion here: #4591 (comment). Priority can't be CSS property because that would allow two highlight pseudos to be stacked differently in different locations within the document, which would violate the "single overlay per highlight for the whole document" principle layed out by the CSS pseudo spec. We also discussed other options for modeling priority, but amongst the options we came up with, a priority attribute on Highlight still seemed to be the best way forward.

Additionally, in the future, when we spec highlight events, it will be beneficial to have the Highlight object and make it an event target. This will make it easier for authors to write event handlers that target any range in a highlight. Ex: The author may want to show a popup with suggestions when the user clicks on spelling errors (implemented via custom highlights) on the page. By making Highlight an event target, the author can write a handler that targets all spelling errors, rather than having to write one per range.

RE the cross-document cases: Yes, we should update the spec to describe how cross-document cases should be handled. I don't believe there are actual use cases for doing this, therefore I'm okay just doing something simple to handle these reasonably. The suggested solution sound reasonable to me: (1) allow highlights to be registered in multiple highlight registries, and (2) when painting highlights registered within a given registry, do not paint ranges that are associated with a different document. In Chromium at least, I don't believe this will be too hard to implement.

[dandclark]

Thanks @sanketj for the extra context! Given that, I'm definitely inclined to go with that simpler solution.

[css-meeting-bot]

The CSS Working Group just discussed Multiple Document Highlights, and agreed to the following:

• RESOLVED: highlights associated with creator document. Throw on mismatches when registering ranges/etc.

The full IRC log of that discussion

<fantasai> Topic: Multiple Document Highlights
<fantasai> github: https://github.com//issues/6417
<fantasai> sanketj: Issue is Highlight API doesn't cases where multiple highlight trees and multiple documents interact
<fantasai> sanketj: e.g. in same-origin iframes, ranges can be passed back and forth
<fantasai> sanketj: Should we be allowed to have highlight covering ranges in different windows?
<fantasai> sanketj: Can a single highlight cross documents?
<fantasai> sanketj: Could reject these cases by throwing
<fantasai> sanketj: I would propose something different
<fantasai> sanketj: I have a 3-part proposal
<GameMaker> Dan Clark is talking, not sanketj
<fantasai> sanketj: 1st, should allow highlight to contain ranges from multiple trees
<fantasai> sanketj: 2nd, allow highlight to be added to multiple registries
<fantasai> sanketj: might mean tracking multiple highlight registries for each highlight
<fantasai> sanketj: When a highlight registry painting, will only paint highlights associated with that document
<TabAtkins> s/sanketj/dandclark/g
<fantasai> sanketj: If it finds a range in another window/iframe, it does not reach over there and paint there. Only paints within its own document
<Rossen_> q?
<fantasai> sanketj: Wanted to get people's thoughts on this proposal
<smfr> q+
<fantasai> smfr: proposal seems to be the most complex version of the solution, allowing highlights to involve multiple documents and allowing them to live in multiple registries
<sanketj> q+
<fantasai> smfr: What's the reason to avoid the simpler solution, of restricting to single document and single registry
<fantasai> sanketj: Philosophical whether to reject early or to be more permissive
<fantasai> sanketj: If I create a highlight but don't give it any ranges, is it associated with that document?
<fantasai> sanketj: ...
<fantasai> sanketj: Do I need to do comparison whether new ranges in another document?
<fantasai> sanketj: Some interesting cases there I don't know how to work through
<emilio> q+
<fantasai> sanketj: We could say something like highlight is associated with document that created it
<fantasai> sanketj: and registry associated with that document
<fantasai> sanketj: and if try to insert range from another document, throws
<TabAtkins> fantasai, stop associating all this with sanketj, it's dandclark
<fantasai> smfr: That would be my preference for the first version
<Rossen_> ack smfr
<TabAtkins> s/sanketj/dandclark/g
<Rossen_> ack sanketj
<fantasai> sanketj: Seems fine, pref for first version
<fantasai> sanketj: Need to define which exception to throw
<fantasai> sanketj: different documents, multiple registries, and multiple ranges to single highlight, need to define exception for each of those
<fantasai> sanketj: might be simpler to allow those cases and don't paint the ranges in different document
<emilio> (sorry, no mic) Strongly prefer associating it with the creator document and throwing on mismatches. There's precedent for this w/ constructable sheets, FontFace, etc
<fantasai> s/first version/first version. If necessary could add these abilities later./
<TabAtkins> Agree with emilio - we already have a decent precedent to only allow same-document usage.
<fantasai> Rossen_: Any other comments/feedback?
<GameMaker> I also agree et. all with throwing
<fantasai> sanketj: If we are throwing, do we want just one exception or separate exceptions for each case? Seem like slightly different operations
<fantasai> TabAtkins: I think they all boil down to same category
<fantasai> TabAtkins: using something in a wrong context
<fantasai> Rossen_: if this proposed path forward works for you, suggest go back to GH and work out the details in terms of exceptions
<fantasai> Rossen_: once all happy, come back and we can resolve
<fantasai> Rossen_: unless you feel strongly on resolving this path forward now
<fantasai> dandclark: Sounds good, thanks
<fantasai> Rossen_: proposal is that we associate with creator document and throw on mismatches
<sanketj> SGTM
<fantasai> Rossen_: any objections?
<dandclark> +1 to resolution
<fantasai> RESOLVED: highlights associated with creator document. Throw on mismatches when registering ranges/etc.
<Rossen_> q

[dandclark]

Right after discussing this I realized there's a problem with the "throw on mismatch" approach that we didn't catch during the meeting. The issue is that a Range can be moved to another Document after it has been added to a Highlight and after the Highlight has been added to the HighlightRegistry.

let iframe = document.createElement("iframe");
document.body.appendChild(iframe);
let r1 = new Range();
r1.setStart(document.body, 0);
r1.setStart(document.body, 1);
let h1 = new Highlight(r1); // doesn't throw; r1 is in the creator document of the Highlight.
CSS.highlights.set("foo", h1); // doesn't throw; Highlight's creator document is the same as the HighlightRegistry's.
r1.setStart(iframe.contentDocument.body, 0); // r1 is now in a different document! But it's too late for highlight APIs to throw.
r1.setEnd(iframe.contentDocument.body, 0);

The above breaks the implied invariant that a HighlightRegistry's Highlights only have Ranges from the document associated with the HighlightRegistry. But, there was no point at which we could have thrown, unless every time we move a Range we do a check for each Highlight that it is a member of.

I can't think of a solution for this, so I think we should revisit the original solution discussed during today's meeting:

1. A Highlight can contain Ranges from multiple document trees.
2. A Highlight can be added to multiple HighlightRegistries.
3. When a HighlightRegistry is painting one of its Highlights, it must only paint the Ranges in the document tree associated
   with that HighlightRegistry.

Or maybe there's a way to work around this problem that I'm not seeing. cc @smfr @emilio @tabatkins @sanketj

[css-meeting-bot]

The CSS Working Group just discussed [css-highlight-api] Specifying behavior for Highlights involving multiple documents, and agreed to the following:

• RESOLVED: revert previous resolution and allow any ranges to be added to a registry. At paint time we only use ranges that are with the originating document

The full IRC log of that discussion

<dael> Topic: [css-highlight-api] Specifying behavior for Highlights involving multiple documents
<dael> github: https://github.com//issues/6417#issuecomment-884332586
<dael> astearns: dandclark can you lead?
<dael> dandclark: This is a rerun of an issue I raised a couple weeks ago. Reached resolution but thought of an issue
<dael> dandclark: Summary is that if we have a couple of same domain iframes have a couple of registries and can add range from wrong window. Resolved to make impossible by throwing when you attempt to add highlight registry from another window
<dael> dandclark: This is trying to establish invarient where they only have ranges from same window. Issue might be is if I add range from same window and I moved to another window I have violated the invarient and it's not clear what to do
<dael> dandclark: No straightfoward way to block unless we check when we move
<TabAtkins> q+
<florian> q?
<dael> dandclark: like to revisit if throwing is right. If we can't maintain this invarient re-open allowing and during painting step they're skipped.
<astearns> ack TabAtkins
<dael> TabAtkins: If we can check what window they're from at painting time I'm not sure why it's problematic to check upon assignemnt to registry
<dael> dandclark: We can, but if at time assigned to registry it's correct but then I can take and position in a different window and it's too late to stop
<florian> q+
<dael> TabAtkins: Oh, didn't realize range could move like that
<dael> dandclark: Codesnip in the issue
<dandclark> https://github.com//issues/6417#issuecomment-884332586
<iank_> ranges are amazing like that.
<dael> florian: I haven't followed this closely, but it reminds me of interaction between ranges and css contain where range had one end outside and one within and the possibility that changing violates containment
<dael> florian: Had considered having in dictionary and ignoring it. I don't think we resolved there and it's still open
<astearns> ack florian
<smfr> q+
<florian> q-
<sanketj> q+
<astearns> ack smfr
<dael> smfr: Do we need to invent a new kind of range that's live but contained in a single document? Cannot move between docs?
<dael> TabAtkins: And restrict to only accept that type?
<dael> smfr: Range would associate to that document and throw if you call with a node from different doc?
<dael> TabAtkins: But we only allow that new type of range with this API?
<dael> smfr: Possibly
<dael> TabAtkins: If we didn't restrict not sure what we gain with the new range type
<dael> smfr: Yeah, only get benefits if you enforce using the new kind of range
<astearns> ack sanketj
<florian> the css-contain issue is : https://github.com//issues/4598
<dael> sanketj: I wanted to echo what florian was saying. Similarities with the contain boundary case where it doesn't make sesne for range boundry to check and it's allowed at paint and we decide then to allow
<dandclark> +1 to what sanketj said :)
<dael> sanketj: This should work same where we allow authors to place ranges where they want but when we paint we only paint those on same originating doc as the highlight registry. That's my preferred solution here
<Rossen_> q?
<GameMaker> +1 for sanketj/florian's solution as well
<Rossen_> q+
<dael> astearns: I think I prefer that solution as well. I can imagine this being whack a mole where we restrict things being added and someone comes up with a method of getting a range into a registry we hadn't thought of
<heycam> q+
<dael> astearns: Seems like it's fitting the problem better
<astearns> ack Rossen_
<dael> Rossen_: I agree with the path forward.
<dael> Rossen_: Before we resolve, sanketj or florian can you remind us what the OM or a11y model behind the ranges? How would they be exposed to assistive tech. xpose collection, only active, etc.?
<dael> florian: I think unresolved in spec. Maybe sanketj knows what was explored impl-wise
<dael> sanketj: I don't think we've reached exploring how to expose. OM-wise the highlight, range objects are OM types we're working with
<dael> Rossen_: I think it would be unfortunate if we reach too far into impl without considering those scenarios. I would suggest to start thinking through these
<dael> florian: You're right. It's been known for a while but it's about time we look into it
<astearns> ack heycam
<dael> heycam: Just realized I thinkw e have another instance of this which is selection object from getSelection. I don't remember what happens but maybe we can see what that does and perhaps do the same
<dael> astearns: Anyone know what happens for selection?
<dael> sanketj: I don't know how selection values update, but nothing blocks from being selected in another document. I'm not sure what happens when it does, though. Nothing stops it from being placed in arbitrary locations
<dael> astearns: previous resolution was throw on mismatches
<dael> astearns: I'm hearing prop is revert previous and allow any ranges to be added to a registry. At paint time we only use ranges that are with the originating document
<dlibby> q+
<dael> astearns: Any discussion on that prop resolution?
<astearns> ack dlibby
<dael> dlibby: We kept mentioning paint time. Is it more about how properties cascade? Or is it really checking when you paint?
<dael> TabAtkins: I suspect it will be some ranges are valid when in right document and painting will only paint valid ranges. And that would let you hook to a11y tools where they only get valid ranges
<dael> florian: And the cascade is for set of ranges for whole highlight so some invalid range doesn't change cascade
<dael> dlibby: that's right, thanks
<dael> astearns: Any more commets?
<dael> astearns: Obj?
<dael> RESOLVED: revert previous resolution and allow any ranges to be added to a registry. At paint time we only use ranges that are with the originating document
<dael> astearns: Thanks for bringing this up again dandclark