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

[css-anchor-1] Can't anchor one popover to another #8165

Closed
xiaochengh opened this issue Nov 30, 2022 · 18 comments · Fixed by #8779
Closed

[css-anchor-1] Can't anchor one popover to another #8165

xiaochengh opened this issue Nov 30, 2022 · 18 comments · Fixed by #8779

Comments

@xiaochengh
Copy link
Contributor

Right now we can't anchor one popover to another, because:

  • Popovers are in the document's top layer, which means they are absolutely positioned with either viewport or ICB as the containing block
  • Then none of them can be an acceptable anchor for another

This means we can't support some very important use cases like nested popover menus.

Not sure exactly which spec to fix (top level, popover or CSS anchor). One possible idea to fix it in CSS is: maybe we can take tree ordering into consideration? If two abspos elements are in the same containing block, the second can anchor to the first.

cc @mfreed7

@bfgeek
Copy link

bfgeek commented Nov 30, 2022

Tree ordering doesn't work for a bunch of edge cases. I'd prefer if there was some logic to prevent the hoisting of the inner popover into the top-layer.

@tabatkins
Copy link
Member

tabatkins commented Nov 30, 2022

So far we've avoided tree order because (a) it's a little more arbitrary (you'd have to order your DOM by how you expect popups to be invoked), and (b) it doesn't give the same "already fully laid out" guarantees that the CB restriction does.

But I think a more targeted fix is possible. We talk about the "top layer" as a single layer, same as the normal document layer, where a lot of things can live together. But if we instead treated it as N top layers, one per object placed "in the top layer", then we can define that earlier layers fully lay out before later ones do, and successive popups will work regardless of their DOM order.

I think that's what we want to do anyway, right? Like, we put things in the top layer to ensure that they display fully over/independently of anything else in the page. If you put a second thing in the top layer you generally want the same guarantees, including things previously placed in the top layer, right? Having independent top-layered things interact seems bad.

@xiaochengh
Copy link
Contributor Author

@tabatkins I think the top layer is already conceptually N top layers, because it's defined as

The top layer is an ordered set of elements, rendered in the order they appear in the set. The last element in the set is rendered last, and thus appears on top.

So your idea should already work with the current top layer spec: by allowing an element that comes later in the top layer anchor to another that comes earlier.

@tabatkins
Copy link
Member

Cool, so as long as it's completely clear that the layout requirement is satisfied (previous elements are laid out prior to later elements), then we're golden.

@mfreed7
Copy link

mfreed7 commented Dec 5, 2022

Tree ordering doesn't work for a bunch of edge cases. I'd prefer if there was some logic to prevent the hoisting of the inner popover into the top-layer.

We can't do this, unfortunately, since that'll break the use case. Two nested popovers - the top one needs to be painted above the bottom one. And they don't have to share a DOM tree, they can be "nested" via invoking attributes.

Cool, so as long as it's completely clear that the layout requirement is satisfied (previous elements are laid out prior to later elements), then we're golden.

+1 to what Xiaocheng said: top layer is really N top layers, in an ordered set. They are rendered in that order. So I would assume (?) that means they do layout in that same order.

I do think this is a very important use case that we should try to support.

@bfgeek
Copy link

bfgeek commented Dec 5, 2022

We can't do this, unfortunately, since that'll break the use case. Two nested popovers - the top one needs to be painted above the bottom one. And they don't have to share a DOM tree, they can be "nested" via invoking attributes.

Don't you want the inner one to be painted above the outer one?

@xiaochengh
Copy link
Contributor Author

Don't you want the inner one to be painted above the outer one?

This is already ensured by the top layer element ordering (which has nothing to do with DOM tree ordering).

@xiaochengh
Copy link
Contributor Author

Following the same idea, we should also allow a top-layer element to anchor to absolutely positioned non-top-layer elements. Otherwise, we can't position the listbox popover of an absolutely-positioned <selectmenu>.

@tabatkins
Copy link
Member

Don't you want the inner one to be painted above the outer one?

Yes, which is why Mason is saying that nested popovers still need to hoist the inner popover to the top layer (in a later "slice") as well. ^_^

Following the same idea, we should also allow a top-layer element to anchor to absolutely positioned non-top-layer elements.

Yes, anything on the base layer (or a lower slice of the top layer) should be a valid anchor for a positioned element in a higher slice.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Dec 7, 2022
Per discussion [1], we should allow a top-layer element to anchor to:
- Non-top-layer absolutely positioned elements
- Elements preceding the current element in the document top-layer

This patch implements it and adds a comprehensive test suite.

The implementation:
- Adds a flag to NGLogicalAnchorQuery::AnchorReference() to allow
  returning invalid (i.e., abspos) anchors, and sets the flag when
  positioning top-layer elements. This is fine because valid anchors
  for a top-layer element are always inserted into the AnchorQuery
  before we layout the top-layer element (ensured by layout tree order)
- Adds a flag to NGPhysicalAnchorQuery::Fragment() to allow returning
  invalid (i.e., abspos) anchors, which is for `anchor-scroll`. The
  result is further validated with layout tree order, so that we don't
  anchor a top-layer element to another element after it in the top
  layer.
- Introduces `ContainingScrollContainerForAnchor()` to correctly get
  the scroll container (nullptr) of a fixed positioned anchor, as this
  case is not possible before this patch

[1] w3c/csswg-drafts#8165

Fixed: 1396436
Fixed: 1385882
Change-Id: I4436f35885e6fede6d780aae633c6525b4100510
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Dec 7, 2022
Per discussion [1], we should allow a top-layer element to anchor to:
- Non-top-layer absolutely positioned elements
- Elements preceding the current element in the document top-layer

This patch implements it and adds a comprehensive test suite.

The implementation:
- Adds a flag to NGLogicalAnchorQuery::AnchorReference() to allow
  returning invalid (i.e., abspos) anchors, and sets the flag when
  positioning top-layer elements. This is fine because valid anchors
  for a top-layer element are always inserted into the AnchorQuery
  before we layout the top-layer element (ensured by layout tree order)
- Adds a flag to NGPhysicalAnchorQuery::Fragment() to allow returning
  invalid (i.e., abspos) anchors, which is for `anchor-scroll`. The
  result is further validated with layout tree order, so that we don't
  anchor a top-layer element to another element after it in the top
  layer.
- Introduces `ContainingScrollContainerForAnchor()` to correctly get
  the scroll container (nullptr) of a fixed positioned anchor, as this
  case is not possible before this patch

[1] w3c/csswg-drafts#8165

Fixed: 1396436
Fixed: 1385882
Change-Id: I4436f35885e6fede6d780aae633c6525b4100510
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Dec 7, 2022
Per discussion [1], we should allow a top-layer element to anchor to:
- Non-top-layer absolutely positioned elements
- Elements preceding the current element in the document top-layer

This patch implements it and adds a comprehensive test suite.

The implementation:
- Adds a flag to NGLogicalAnchorQuery::AnchorReference() to allow
  returning invalid (i.e., abspos) anchors, and sets the flag when
  positioning top-layer elements. This is fine because valid anchors
  for a top-layer element are always inserted into the AnchorQuery
  before we layout the top-layer element (ensured by layout tree order)
- Adds a flag to NGPhysicalAnchorQuery::Fragment() to allow returning
  invalid (i.e., abspos) anchors, which is for `anchor-scroll`. The
  result is further validated with layout tree order, so that we don't
  anchor a top-layer element to another element after it in the top
  layer.
- Introduces `ContainingScrollContainerForAnchor()` to correctly get
  the scroll container (nullptr) of a fixed positioned anchor, as this
  case is not possible before this patch

[1] w3c/csswg-drafts#8165

Fixed: 1396436
Fixed: 1385882
Change-Id: I4436f35885e6fede6d780aae633c6525b4100510
aarongable pushed a commit to chromium/chromium that referenced this issue Dec 8, 2022
Per discussion [1], we should allow a top-layer element to anchor to:
- Non-top-layer absolutely positioned elements
- Elements preceding the current element in the document top-layer

This patch implements it and adds a comprehensive test suite.

The implementation:
- Adds a flag to NGLogicalAnchorQuery::AnchorReference() to allow
  returning invalid (i.e., abspos) anchors, and sets the flag when
  positioning top-layer elements. This is fine because valid anchors
  for a top-layer element are always inserted into the AnchorQuery
  before we layout the top-layer element (ensured by layout tree order)
- Adds a flag to NGPhysicalAnchorQuery::Fragment() to allow returning
  invalid (i.e., abspos) anchors, which is for `anchor-scroll`. The
  result is further validated with layout tree order, so that we don't
  anchor a top-layer element to another element after it in the top
  layer.
- Introduces `ContainingScrollContainerForAnchor()` to correctly get
  the scroll container (nullptr) of a fixed positioned anchor, as this
  case is not possible before this patch

[1] w3c/csswg-drafts#8165

Fixed: 1396436
Fixed: 1385882
Change-Id: I4436f35885e6fede6d780aae633c6525b4100510
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4083692
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Auto-Submit: Xiaocheng Hu <xiaochengh@chromium.org>
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1080999}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Dec 8, 2022
Per discussion [1], we should allow a top-layer element to anchor to:
- Non-top-layer absolutely positioned elements
- Elements preceding the current element in the document top-layer

This patch implements it and adds a comprehensive test suite.

The implementation:
- Adds a flag to NGLogicalAnchorQuery::AnchorReference() to allow
  returning invalid (i.e., abspos) anchors, and sets the flag when
  positioning top-layer elements. This is fine because valid anchors
  for a top-layer element are always inserted into the AnchorQuery
  before we layout the top-layer element (ensured by layout tree order)
- Adds a flag to NGPhysicalAnchorQuery::Fragment() to allow returning
  invalid (i.e., abspos) anchors, which is for `anchor-scroll`. The
  result is further validated with layout tree order, so that we don't
  anchor a top-layer element to another element after it in the top
  layer.
- Introduces `ContainingScrollContainerForAnchor()` to correctly get
  the scroll container (nullptr) of a fixed positioned anchor, as this
  case is not possible before this patch

[1] w3c/csswg-drafts#8165

Fixed: 1396436
Fixed: 1385882
Change-Id: I4436f35885e6fede6d780aae633c6525b4100510
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4083692
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Auto-Submit: Xiaocheng Hu <xiaochengh@chromium.org>
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1080999}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Dec 8, 2022
Per discussion [1], we should allow a top-layer element to anchor to:
- Non-top-layer absolutely positioned elements
- Elements preceding the current element in the document top-layer

This patch implements it and adds a comprehensive test suite.

The implementation:
- Adds a flag to NGLogicalAnchorQuery::AnchorReference() to allow
  returning invalid (i.e., abspos) anchors, and sets the flag when
  positioning top-layer elements. This is fine because valid anchors
  for a top-layer element are always inserted into the AnchorQuery
  before we layout the top-layer element (ensured by layout tree order)
- Adds a flag to NGPhysicalAnchorQuery::Fragment() to allow returning
  invalid (i.e., abspos) anchors, which is for `anchor-scroll`. The
  result is further validated with layout tree order, so that we don't
  anchor a top-layer element to another element after it in the top
  layer.
- Introduces `ContainingScrollContainerForAnchor()` to correctly get
  the scroll container (nullptr) of a fixed positioned anchor, as this
  case is not possible before this patch

[1] w3c/csswg-drafts#8165

Fixed: 1396436
Fixed: 1385882
Change-Id: I4436f35885e6fede6d780aae633c6525b4100510
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4083692
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Auto-Submit: Xiaocheng Hu <xiaochengh@chromium.org>
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1080999}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Dec 12, 2022
…ts to anchor to abspos elements, a=testonly

Automatic update from web-platform-tests
[anchor-position] Allow top-layer elements to anchor to abspos elements

Per discussion [1], we should allow a top-layer element to anchor to:
- Non-top-layer absolutely positioned elements
- Elements preceding the current element in the document top-layer

This patch implements it and adds a comprehensive test suite.

The implementation:
- Adds a flag to NGLogicalAnchorQuery::AnchorReference() to allow
  returning invalid (i.e., abspos) anchors, and sets the flag when
  positioning top-layer elements. This is fine because valid anchors
  for a top-layer element are always inserted into the AnchorQuery
  before we layout the top-layer element (ensured by layout tree order)
- Adds a flag to NGPhysicalAnchorQuery::Fragment() to allow returning
  invalid (i.e., abspos) anchors, which is for `anchor-scroll`. The
  result is further validated with layout tree order, so that we don't
  anchor a top-layer element to another element after it in the top
  layer.
- Introduces `ContainingScrollContainerForAnchor()` to correctly get
  the scroll container (nullptr) of a fixed positioned anchor, as this
  case is not possible before this patch

[1] w3c/csswg-drafts#8165

Fixed: 1396436
Fixed: 1385882
Change-Id: I4436f35885e6fede6d780aae633c6525b4100510
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4083692
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Auto-Submit: Xiaocheng Hu <xiaochengh@chromium.org>
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1080999}

--

wpt-commits: 85ef51bdd55cea3fe87648edcedf1501ae6834dd
wpt-pr: 37366
BruceDai pushed a commit to BruceDai/wpt that referenced this issue Dec 13, 2022
Per discussion [1], we should allow a top-layer element to anchor to:
- Non-top-layer absolutely positioned elements
- Elements preceding the current element in the document top-layer

This patch implements it and adds a comprehensive test suite.

The implementation:
- Adds a flag to NGLogicalAnchorQuery::AnchorReference() to allow
  returning invalid (i.e., abspos) anchors, and sets the flag when
  positioning top-layer elements. This is fine because valid anchors
  for a top-layer element are always inserted into the AnchorQuery
  before we layout the top-layer element (ensured by layout tree order)
- Adds a flag to NGPhysicalAnchorQuery::Fragment() to allow returning
  invalid (i.e., abspos) anchors, which is for `anchor-scroll`. The
  result is further validated with layout tree order, so that we don't
  anchor a top-layer element to another element after it in the top
  layer.
- Introduces `ContainingScrollContainerForAnchor()` to correctly get
  the scroll container (nullptr) of a fixed positioned anchor, as this
  case is not possible before this patch

[1] w3c/csswg-drafts#8165

Fixed: 1396436
Fixed: 1385882
Change-Id: I4436f35885e6fede6d780aae633c6525b4100510
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4083692
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Auto-Submit: Xiaocheng Hu <xiaochengh@chromium.org>
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1080999}
jamienicol pushed a commit to jamienicol/gecko that referenced this issue Dec 14, 2022
…ts to anchor to abspos elements, a=testonly

Automatic update from web-platform-tests
[anchor-position] Allow top-layer elements to anchor to abspos elements

Per discussion [1], we should allow a top-layer element to anchor to:
- Non-top-layer absolutely positioned elements
- Elements preceding the current element in the document top-layer

This patch implements it and adds a comprehensive test suite.

The implementation:
- Adds a flag to NGLogicalAnchorQuery::AnchorReference() to allow
  returning invalid (i.e., abspos) anchors, and sets the flag when
  positioning top-layer elements. This is fine because valid anchors
  for a top-layer element are always inserted into the AnchorQuery
  before we layout the top-layer element (ensured by layout tree order)
- Adds a flag to NGPhysicalAnchorQuery::Fragment() to allow returning
  invalid (i.e., abspos) anchors, which is for `anchor-scroll`. The
  result is further validated with layout tree order, so that we don't
  anchor a top-layer element to another element after it in the top
  layer.
- Introduces `ContainingScrollContainerForAnchor()` to correctly get
  the scroll container (nullptr) of a fixed positioned anchor, as this
  case is not possible before this patch

[1] w3c/csswg-drafts#8165

Fixed: 1396436
Fixed: 1385882
Change-Id: I4436f35885e6fede6d780aae633c6525b4100510
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4083692
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Auto-Submit: Xiaocheng Hu <xiaochengh@chromium.org>
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1080999}

--

wpt-commits: 85ef51bdd55cea3fe87648edcedf1501ae6834dd
wpt-pr: 37366
@xiaochengh
Copy link
Contributor Author

xiaochengh commented Jan 6, 2023

Is this already fixed by a5eb357?

(And please add the css-anchor-1 label before closing)

@kizu
Copy link
Member

kizu commented Mar 15, 2023

Disclaimer and additional links

I'm submitting my feedback following my experiments with the current implementation of anchor positioning in Chrome Canary.

I wrote an article about my experiments, but decided to fill most of my feedback as separate issues here.

A quick summary of related links:

I'm using this issue instead of creating a new one in order not to duplicate things. The issue topic that I wanted to give — “[css-anchor-1] Alternative acceptance criteria”. I did not read in details the existing discussion, and I think I don't have all the context around it, so if what I would describe is/would already be possible with the current specs — let me know!

While most of my experiments were ok with the way the current acceptance criteria work, one of them was heavily limited by it.

The experiment in question: https://kizu.dev/anchor-positioning-experiments/#sidenotes-layout

In this experiment I wanted to attach one absolutely positioned element to another absolutely positioned element, and do this in a chain. This is very similar to cases with multiple nested popovers.

The workaround I found with wrapping things in multiple wrappers and placing the absolutely positioned elements in each of them in a descending order kinda works, but not feasible in any manner for any kind of production code.

While I find the current way things work acceptable (after all, most my other experiments were not blocked by this limitation), I would really-really want to be able to chain-link absolutely positioned elements effortlessly.

What if we could define an alternative acceptance criteria via some additional CSS property? Or maybe even just add it to the current list of them, as the last fallback?

Of course, what I'm talking is — the DOM order. With it there can never be any circularity issues, and while it is not perfect, it would allow us to solve a lot of things.

Can we add it to the existing list of criteria in a way where “if nothing above applies, the element that goes later in the DOM could target the one before it”. This way, we would get the best of two worlds — all the existing ways we can attach things would keep working, but then we would also get an ability to chain-link stuff easily.

The argument about “you'd have to order your DOM by how you expect popups to be invoked” does not work, as the current workaround already expects even more drastic DOM structure, and in absence of a working solution, people would actually use it. Would we want to have 20 wrapping divs for an article with 20 footnotes? Or to create N wrapping divs just so we could chain-link popovers, putting them in different layers?

DOM order is easy to work with, is intuitive and works for the existing chain-linking cases.

My second alternative/addition I wanted to propose — make things works similarly to the way variables can fail — as soon as the circularity is detected, all related attachments should just stop working. Similarly how --foo: --bar; --bar: --baz; --baz: --foo; results in all three being invalid, chain-linking elements in a loop could make all of them invalid.

Though, I imagine, this way of handling things is nearly impossible if we want to make things performant?

@xiaochengh
Copy link
Contributor Author

Had an offline discussion with @tabatkins

We generally agreed that we should relax the anchor criteria and allow anchoring to abspos elements that come earlier in the tree order. This also covers the top-layer case naturally.

I'll also do some prototyping in Chromium to see how it's going to work.

@xiaochengh
Copy link
Contributor Author

I made a PR for the idea.

Btw, when there are multiple anchors with the same name, maybe we should return the last acceptable anchor instead of the first one (as we currently do)?

This allows easy chaining of anchor positioned elements with the same name, a case @tabatkins mentioned in a chat:

<style>
.anchored {
  anchor-name: --a;
  position: absolute;
  top: anchor(--a bottom);
}
</style>
<div class=anchored></div>
<div class=anchored></div>
<div class=anchored></div>
<div class=anchored></div>

Each .anchored will be anchored below the previous one if we use the last acceptable anchor instead of the first one.

@kizu
Copy link
Member

kizu commented Apr 29, 2023

I made a PR for the idea.

This is great, thank you for working on this! Can't wait to try it :)

Btw, when there are multiple anchors with the same name, maybe we should return the last acceptable anchor instead of the first one (as we currently do)?

This sounds really nice! I mean, ideally, it would be better to get the closest element to the one we're anchoring, but I imagine it is much harder to both define and implement? If so, then using the last one instead is fine, and would be very useful for the chaining use-case, while I can't think of any use-case where anchoring to the first element would help.

@tabatkins
Copy link
Member

tabatkins commented May 4, 2023

Btw, when there are multiple anchors with the same name, maybe we should return the last acceptable anchor instead of the first one (as we currently do)?

Oooh, yes, that sounds much better. I went for "first" just because I needed an answer and figured it was fairly arbitrary, but you're right that now that we allow elements at the same level to see each other, "last" actually solves a reasonable use-case.


A little more precisely, the use-case I wanted was "anchor a sidebar note to its anchor in the main text, or below the preceding sidebar note, if they would overlap". So it would be like:

<style>
p:has(+ aside.sidebar) {
  anchor-name: --sidebar-anchor;
}
aside.sidebar {
  anchor-name: --sidebar-note;
  position: absolute;
  top: max(anchor(--sidebar-note bottom), anchor(--sidebar-anchor top));
}
</style>
<p>Some text
<aside class=sidebar>...</aside>
<p>Some more text
<aside class=sidebar>...</aside>

Now the layout is good regardless of how tall the paragraphs or notes are; the notes will try to anchor to their preceding paragraph, and if a previous note is too tall is taller than its corresponding paragraph, the next paragraph's note will get pushed down, cascading all the way down the document as required.

@tabatkins
Copy link
Member

Btw, the fact that I was able to write the preceding example without adding a single piece of per-item identifying information absolutely kicks ass.

@xiaochengh
Copy link
Contributor Author

Thanks! #8779 only relaxes the anchor criteria. I'll upload another PR to change "first" to "last".

@kizu
Copy link
Member

kizu commented May 4, 2023

Btw, the fact that I was able to write the preceding example without adding a single piece of per-item identifying information absolutely kicks ass.

Yep, this is very very cool! Much cleaner than what I had to do for my version of this exact use-case based on the initial implementation, haha.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue May 9, 2023
As decided in w3c/csswg-drafts#8165, anchoring
to out-of-flow anchors should be generally allowed, as long as the
anchor comes earlier in the tree order. This patch implements the
change.

Since we are already calling NGLogicalAnchorQuery::Set() for out-of-flow
anchors, this patch only changes the insertion algorithm to not filter
then out, and the query algorithm to consider out-of-flow anchors.

Existing support of top-layer target elements is covered by this change,
and hence its special handling code is remove.

Bug: 1442752
Change-Id: I81e80a143447369d1fb3c5a4aee587760cfb913f
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue May 9, 2023
As decided in w3c/csswg-drafts#8165, anchoring
to out-of-flow anchors should be generally allowed, as long as the
anchor comes earlier in the tree order. This patch implements the
change.

Since we are already calling NGLogicalAnchorQuery::Set() for out-of-flow
anchors, this patch only changes the insertion algorithm to not filter
then out, and the query algorithm to consider out-of-flow anchors.

Existing support of top-layer target elements is covered by this change,
and hence its special handling code is remove.

Bug: 1442752
Change-Id: I81e80a143447369d1fb3c5a4aee587760cfb913f
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue May 10, 2023
As decided in w3c/csswg-drafts#8165, anchoring
to out-of-flow anchors should be generally allowed, as long as the
anchor comes earlier in the tree order. This patch implements the
change.

Since we are already calling NGLogicalAnchorQuery::Set() for out-of-flow
anchors, this patch only changes the insertion algorithm to not filter
then out, and the query algorithm to consider out-of-flow anchors.

Existing support of top-layer target elements is covered by this change,
and hence its special handling code is remove.

Bug: 1442752
Change-Id: I81e80a143447369d1fb3c5a4aee587760cfb913f
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue May 12, 2023
As decided in w3c/csswg-drafts#8165, anchoring
to out-of-flow anchors should be generally allowed, as long as the
anchor comes earlier in the tree order. This patch implements the
change.

Since we are already calling NGLogicalAnchorQuery::Set() for out-of-flow
anchors, this patch only changes the insertion algorithm to not filter
then out, and the query algorithm to consider out-of-flow anchors.

Existing support of top-layer target elements is covered by this change,
and hence its special handling code is remove.

This patch also reveals an existing issue of discovering OOF anchors
in block fragmentation, which caused the failure of a test. As this is
an edge case, we only track the failure with the block fragmentation
meta bug crbug.com/1378460.

Bug: 1378460, 1442752
Change-Id: I81e80a143447369d1fb3c5a4aee587760cfb913f
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue May 12, 2023
As decided in w3c/csswg-drafts#8165, anchoring
to out-of-flow anchors should be generally allowed, as long as the
anchor comes earlier in the tree order. This patch implements the
change.

Since we are already calling NGLogicalAnchorQuery::Set() for out-of-flow
anchors, this patch only changes the insertion algorithm to not filter
then out, and the query algorithm to consider out-of-flow anchors.

Existing support of top-layer target elements is covered by this change,
and hence its special handling code is remove.

This patch also reveals an existing issue of discovering OOF anchors
in block fragmentation, which caused the failure of some tests. As
this is an edge case, we only track the failure with the block fragmentation meta bug crbug.com/1378460.

Bug: 1378460, 1442752
Change-Id: I81e80a143447369d1fb3c5a4aee587760cfb913f
aarongable pushed a commit to chromium/chromium that referenced this issue May 17, 2023
As decided in w3c/csswg-drafts#8165, anchoring
to out-of-flow anchors should be generally allowed, as long as the
anchor comes earlier in the tree order. This patch implements the
change.

Since we are already calling NGLogicalAnchorQuery::Set() for out-of-flow
anchors, this patch only changes the insertion algorithm to not filter
then out, and the query algorithm to consider out-of-flow anchors.

Existing support of top-layer target elements is covered by this change,
and hence its special handling code is remove.

This patch also reveals an existing issue of discovering OOF anchors
in block fragmentation, which caused the failure of some tests. As
this is an edge case, we only track the failure with the block fragmentation meta bug crbug.com/1378460.

Bug: 1378460, 1442752
Change-Id: I81e80a143447369d1fb3c5a4aee587760cfb913f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4518512
Reviewed-by: Koji Ishii <kojii@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1145042}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue May 17, 2023
As decided in w3c/csswg-drafts#8165, anchoring
to out-of-flow anchors should be generally allowed, as long as the
anchor comes earlier in the tree order. This patch implements the
change.

Since we are already calling NGLogicalAnchorQuery::Set() for out-of-flow
anchors, this patch only changes the insertion algorithm to not filter
then out, and the query algorithm to consider out-of-flow anchors.

Existing support of top-layer target elements is covered by this change,
and hence its special handling code is remove.

This patch also reveals an existing issue of discovering OOF anchors
in block fragmentation, which caused the failure of some tests. As
this is an edge case, we only track the failure with the block fragmentation meta bug crbug.com/1378460.

Bug: 1378460, 1442752
Change-Id: I81e80a143447369d1fb3c5a4aee587760cfb913f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4518512
Reviewed-by: Koji Ishii <kojii@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1145042}
aarongable pushed a commit to chromium/chromium that referenced this issue May 17, 2023
As decided in w3c/csswg-drafts#8165, when
resolving anchor name conflicts, we should return the last acceptable
anchor element instead of the first one. This patch implements the
change.

The biggest change is that NGLogicalAnchorQuery references are now
a list sorted in reversed tree order, so that the query algorithm
remains the same as before: find the first entry that comes earlier
in tree order.

In in-order vs. out-of-order flag in NGLogicalAnchorQuery creation has
become not very useful because an in-order call simply prepends an
entry to the reference list. Hence this patch removes the flag and the
related logic.

Bug: 1442752
Change-Id: I3f28f1b66097fa7a0a333080dd0f29e62f417b71
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4518226
Reviewed-by: Koji Ishii <kojii@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1145068}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue May 17, 2023
As decided in w3c/csswg-drafts#8165, anchoring
to out-of-flow anchors should be generally allowed, as long as the
anchor comes earlier in the tree order. This patch implements the
change.

Since we are already calling NGLogicalAnchorQuery::Set() for out-of-flow
anchors, this patch only changes the insertion algorithm to not filter
then out, and the query algorithm to consider out-of-flow anchors.

Existing support of top-layer target elements is covered by this change,
and hence its special handling code is remove.

This patch also reveals an existing issue of discovering OOF anchors
in block fragmentation, which caused the failure of some tests. As
this is an edge case, we only track the failure with the block fragmentation meta bug crbug.com/1378460.

Bug: 1378460, 1442752
Change-Id: I81e80a143447369d1fb3c5a4aee587760cfb913f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4518512
Reviewed-by: Koji Ishii <kojii@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1145042}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue May 17, 2023
As decided in w3c/csswg-drafts#8165, when
resolving anchor name conflicts, we should return the last acceptable
anchor element instead of the first one. This patch implements the
change.

The biggest change is that NGLogicalAnchorQuery references are now
a list sorted in reversed tree order, so that the query algorithm
remains the same as before: find the first entry that comes earlier
in tree order.

In in-order vs. out-of-order flag in NGLogicalAnchorQuery creation has
become not very useful because an in-order call simply prepends an
entry to the reference list. Hence this patch removes the flag and the
related logic.

Bug: 1442752
Change-Id: I3f28f1b66097fa7a0a333080dd0f29e62f417b71
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4518226
Reviewed-by: Koji Ishii <kojii@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1145068}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue May 17, 2023
As decided in w3c/csswg-drafts#8165, when
resolving anchor name conflicts, we should return the last acceptable
anchor element instead of the first one. This patch implements the
change.

The biggest change is that NGLogicalAnchorQuery references are now
a list sorted in reversed tree order, so that the query algorithm
remains the same as before: find the first entry that comes earlier
in tree order.

In in-order vs. out-of-order flag in NGLogicalAnchorQuery creation has
become not very useful because an in-order call simply prepends an
entry to the reference list. Hence this patch removes the flag and the
related logic.

Bug: 1442752
Change-Id: I3f28f1b66097fa7a0a333080dd0f29e62f417b71
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4518226
Reviewed-by: Koji Ishii <kojii@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1145068}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue May 22, 2023
…-flow anchors, a=testonly

Automatic update from web-platform-tests
[anchor-position] Generally allow out-of-flow anchors

As decided in w3c/csswg-drafts#8165, anchoring
to out-of-flow anchors should be generally allowed, as long as the
anchor comes earlier in the tree order. This patch implements the
change.

Since we are already calling NGLogicalAnchorQuery::Set() for out-of-flow
anchors, this patch only changes the insertion algorithm to not filter
then out, and the query algorithm to consider out-of-flow anchors.

Existing support of top-layer target elements is covered by this change,
and hence its special handling code is remove.

This patch also reveals an existing issue of discovering OOF anchors
in block fragmentation, which caused the failure of some tests. As
this is an edge case, we only track the failure with the block fragmentation meta bug crbug.com/1378460.

Bug: 1378460, 1442752
Change-Id: I81e80a143447369d1fb3c5a4aee587760cfb913f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4518512
Reviewed-by: Koji Ishii <kojii@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1145042}

--

wpt-commits: bc5a9c39ebb72a8f8b7859cb0436fa65f3bd98b2
wpt-pr: 39912
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue May 22, 2023
…able anchor element, a=testonly

Automatic update from web-platform-tests
[anchor-position] Return the last acceptable anchor element

As decided in w3c/csswg-drafts#8165, when
resolving anchor name conflicts, we should return the last acceptable
anchor element instead of the first one. This patch implements the
change.

The biggest change is that NGLogicalAnchorQuery references are now
a list sorted in reversed tree order, so that the query algorithm
remains the same as before: find the first entry that comes earlier
in tree order.

In in-order vs. out-of-order flag in NGLogicalAnchorQuery creation has
become not very useful because an in-order call simply prepends an
entry to the reference list. Hence this patch removes the flag and the
related logic.

Bug: 1442752
Change-Id: I3f28f1b66097fa7a0a333080dd0f29e62f417b71
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4518226
Reviewed-by: Koji Ishii <kojii@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1145068}

--

wpt-commits: cc9dbcc9ff96216f57af50b37231a09dea806185
wpt-pr: 40048
ErichDonGubler pushed a commit to erichdongubler-mozilla/firefox that referenced this issue May 23, 2023
…-flow anchors, a=testonly

Automatic update from web-platform-tests
[anchor-position] Generally allow out-of-flow anchors

As decided in w3c/csswg-drafts#8165, anchoring
to out-of-flow anchors should be generally allowed, as long as the
anchor comes earlier in the tree order. This patch implements the
change.

Since we are already calling NGLogicalAnchorQuery::Set() for out-of-flow
anchors, this patch only changes the insertion algorithm to not filter
then out, and the query algorithm to consider out-of-flow anchors.

Existing support of top-layer target elements is covered by this change,
and hence its special handling code is remove.

This patch also reveals an existing issue of discovering OOF anchors
in block fragmentation, which caused the failure of some tests. As
this is an edge case, we only track the failure with the block fragmentation meta bug crbug.com/1378460.

Bug: 1378460, 1442752
Change-Id: I81e80a143447369d1fb3c5a4aee587760cfb913f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4518512
Reviewed-by: Koji Ishii <kojii@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1145042}

--

wpt-commits: bc5a9c39ebb72a8f8b7859cb0436fa65f3bd98b2
wpt-pr: 39912
ErichDonGubler pushed a commit to erichdongubler-mozilla/firefox that referenced this issue May 23, 2023
…able anchor element, a=testonly

Automatic update from web-platform-tests
[anchor-position] Return the last acceptable anchor element

As decided in w3c/csswg-drafts#8165, when
resolving anchor name conflicts, we should return the last acceptable
anchor element instead of the first one. This patch implements the
change.

The biggest change is that NGLogicalAnchorQuery references are now
a list sorted in reversed tree order, so that the query algorithm
remains the same as before: find the first entry that comes earlier
in tree order.

In in-order vs. out-of-order flag in NGLogicalAnchorQuery creation has
become not very useful because an in-order call simply prepends an
entry to the reference list. Hence this patch removes the flag and the
related logic.

Bug: 1442752
Change-Id: I3f28f1b66097fa7a0a333080dd0f29e62f417b71
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4518226
Reviewed-by: Koji Ishii <kojii@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1145068}

--

wpt-commits: cc9dbcc9ff96216f57af50b37231a09dea806185
wpt-pr: 40048
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
6 participants
@kizu @tabatkins @bfgeek @xiaochengh @mfreed7 and others