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-position-3] Define containing block for stickypos internal table elements #5020

Open
fantasai opened this issue Apr 29, 2020 · 7 comments

Comments

@fantasai
Copy link
Collaborator

Internal table elements don't really have a "containing block" defined per se, but one is needed to intersect with the viewport for sticky positioning. So we need to define that, but also what should it be? The table grid box? the table row box? something else?

@tabatkins
Copy link
Member

I think the answer should be that the containing block for rows and cells should be their row group, and for row groups it shoudl be the table grid box.

This way you can get sticky headers with thead { position: sticky; top: 0; }, but also get individual sticky rows that can get "carried away" by judicious use of tbodys, just like sticky headings + section.

@atotic
Copy link
Contributor

atotic commented Nov 17, 2020

It's the "sticky failures" day at TablesNG manor.
Here is a test case: https://wptest.center/#/1vb898 (my first time using wptest)
It applies sticky constraints table parts, scrolls, and observes part position.

I am testing for the most common behavior in current browsers which is:
"Table wrapper fragment is the sticky container for all table parts"

Results:

// FF 82: pass
// Chrome legacy: TD pass, TR/TBODY fail, sticky treated as static
// Edge 17:TD pass, TR/TBODY fail, sticky treated as static
// Safari: TD/TR fail creatively, TBODY pass.

For TD, sticky container being Table wrapper matches widest current behavior.
If we adopt this for TD, makes sense to do it for TR too.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Nov 17, 2020
Legacy used Table as a sticky container for TD.

Bring TablesNG in sync with legacy, and make
table rows also use table as a sticky container.

Standard not settled yet, discussion at:
w3c/csswg-drafts#5020

Makes 2 layout tests pass:

external/wpt/css/css-position/sticky/position-sticky-table-th-bottom.html
external/wpt/css/css-position/sticky/position-sticky-table-th-top.html

Makes 3 unit tests pass
LayoutBoxModelObjectTest.StickyPositionComplexTableNesting (layout_box_model_object_test.cc:886)
LayoutBoxModelObjectTest.StickyPositionNestedStickyTable (layout_box_model_object_test.cc:818)
LayoutBoxModelObjectTest.StickyPositionTableContainers (layout_box_model_object_test.cc:349)

Bug: 958381
Change-Id: If099efe32a6a23154b53bf96981f7176030cd52c
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Nov 17, 2020
Legacy used Table as a sticky container for TD.

Bring TablesNG in sync with legacy, and make
table rows also use table as a sticky container.

Standard not settled yet, discussion at:
w3c/csswg-drafts#5020

Makes 2 layout tests pass:

external/wpt/css/css-position/sticky/position-sticky-table-th-bottom.html
external/wpt/css/css-position/sticky/position-sticky-table-th-top.html

Makes 3 unit tests pass
LayoutBoxModelObjectTest.StickyPositionComplexTableNesting (layout_box_model_object_test.cc:886)
LayoutBoxModelObjectTest.StickyPositionNestedStickyTable (layout_box_model_object_test.cc:818)
LayoutBoxModelObjectTest.StickyPositionTableContainers (layout_box_model_object_test.cc:349)

Bug: 958381
Change-Id: If099efe32a6a23154b53bf96981f7176030cd52c
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Nov 18, 2020
Legacy used Table as a sticky container for TD.

Bring TablesNG in sync with legacy, and make
table rows also use table as a sticky container.

Standard not settled yet, discussion at:
w3c/csswg-drafts#5020

Makes 2 layout tests pass:

external/wpt/css/css-position/sticky/position-sticky-table-th-bottom.html
external/wpt/css/css-position/sticky/position-sticky-table-th-top.html

Makes 3 unit tests pass
LayoutBoxModelObjectTest.StickyPositionComplexTableNesting (layout_box_model_object_test.cc:886)
LayoutBoxModelObjectTest.StickyPositionNestedStickyTable (layout_box_model_object_test.cc:818)
LayoutBoxModelObjectTest.StickyPositionTableContainers (layout_box_model_object_test.cc:349)

Bug: 958381
Change-Id: If099efe32a6a23154b53bf96981f7176030cd52c
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Nov 19, 2020
Legacy used Table as a sticky container for TD.

Bring TablesNG in sync with legacy, and make
table rows also use table as a sticky container.

Standard not settled yet, discussion at:
w3c/csswg-drafts#5020

Makes 2 layout tests pass:

external/wpt/css/css-position/sticky/position-sticky-table-th-bottom.html
external/wpt/css/css-position/sticky/position-sticky-table-th-top.html

Makes 3 unit tests pass
LayoutBoxModelObjectTest.StickyPositionComplexTableNesting (layout_box_model_object_test.cc:886)
LayoutBoxModelObjectTest.StickyPositionNestedStickyTable (layout_box_model_object_test.cc:818)
LayoutBoxModelObjectTest.StickyPositionTableContainers (layout_box_model_object_test.cc:349)

Bug: 958381
Change-Id: If099efe32a6a23154b53bf96981f7176030cd52c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2542737
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Commit-Queue: Aleks Totic <atotic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#828994}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Nov 19, 2020
Legacy used Table as a sticky container for TD.

Bring TablesNG in sync with legacy, and make
table rows also use table as a sticky container.

Standard not settled yet, discussion at:
w3c/csswg-drafts#5020

Makes 2 layout tests pass:

external/wpt/css/css-position/sticky/position-sticky-table-th-bottom.html
external/wpt/css/css-position/sticky/position-sticky-table-th-top.html

Makes 3 unit tests pass
LayoutBoxModelObjectTest.StickyPositionComplexTableNesting (layout_box_model_object_test.cc:886)
LayoutBoxModelObjectTest.StickyPositionNestedStickyTable (layout_box_model_object_test.cc:818)
LayoutBoxModelObjectTest.StickyPositionTableContainers (layout_box_model_object_test.cc:349)

Bug: 958381
Change-Id: If099efe32a6a23154b53bf96981f7176030cd52c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2542737
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Commit-Queue: Aleks Totic <atotic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#828994}
blueboxd pushed a commit to blueboxd/chromium-legacy that referenced this issue Nov 19, 2020
Legacy used Table as a sticky container for TD.

Bring TablesNG in sync with legacy, and make
table rows also use table as a sticky container.

Standard not settled yet, discussion at:
w3c/csswg-drafts#5020

Makes 2 layout tests pass:

external/wpt/css/css-position/sticky/position-sticky-table-th-bottom.html
external/wpt/css/css-position/sticky/position-sticky-table-th-top.html

Makes 3 unit tests pass
LayoutBoxModelObjectTest.StickyPositionComplexTableNesting (layout_box_model_object_test.cc:886)
LayoutBoxModelObjectTest.StickyPositionNestedStickyTable (layout_box_model_object_test.cc:818)
LayoutBoxModelObjectTest.StickyPositionTableContainers (layout_box_model_object_test.cc:349)

Bug: 958381
Change-Id: If099efe32a6a23154b53bf96981f7176030cd52c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2542737
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Commit-Queue: Aleks Totic <atotic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#828994}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Nov 26, 2020
Automatic update from web-platform-tests
[TablesNG] Sticky position

Legacy used Table as a sticky container for TD.

Bring TablesNG in sync with legacy, and make
table rows also use table as a sticky container.

Standard not settled yet, discussion at:
w3c/csswg-drafts#5020

Makes 2 layout tests pass:

external/wpt/css/css-position/sticky/position-sticky-table-th-bottom.html
external/wpt/css/css-position/sticky/position-sticky-table-th-top.html

Makes 3 unit tests pass
LayoutBoxModelObjectTest.StickyPositionComplexTableNesting (layout_box_model_object_test.cc:886)
LayoutBoxModelObjectTest.StickyPositionNestedStickyTable (layout_box_model_object_test.cc:818)
LayoutBoxModelObjectTest.StickyPositionTableContainers (layout_box_model_object_test.cc:349)

Bug: 958381
Change-Id: If099efe32a6a23154b53bf96981f7176030cd52c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2542737
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Commit-Queue: Aleks Totic <atotic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#828994}

--

wpt-commits: 1b52cf975cb73d97848b9dee0a1609fe46aebaea
wpt-pr: 26549
sidvishnoi pushed a commit to sidvishnoi/gecko-webmonetization that referenced this issue Nov 27, 2020
Automatic update from web-platform-tests
[TablesNG] Sticky position

Legacy used Table as a sticky container for TD.

Bring TablesNG in sync with legacy, and make
table rows also use table as a sticky container.

Standard not settled yet, discussion at:
w3c/csswg-drafts#5020

Makes 2 layout tests pass:

external/wpt/css/css-position/sticky/position-sticky-table-th-bottom.html
external/wpt/css/css-position/sticky/position-sticky-table-th-top.html

Makes 3 unit tests pass
LayoutBoxModelObjectTest.StickyPositionComplexTableNesting (layout_box_model_object_test.cc:886)
LayoutBoxModelObjectTest.StickyPositionNestedStickyTable (layout_box_model_object_test.cc:818)
LayoutBoxModelObjectTest.StickyPositionTableContainers (layout_box_model_object_test.cc:349)

Bug: 958381
Change-Id: If099efe32a6a23154b53bf96981f7176030cd52c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2542737
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Commit-Queue: Aleks Totic <atotic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#828994}

--

wpt-commits: 1b52cf975cb73d97848b9dee0a1609fe46aebaea
wpt-pr: 26549
sylvestre pushed a commit to sylvestre/gecko-dev that referenced this issue Nov 28, 2020
Automatic update from web-platform-tests
[TablesNG] Sticky position

Legacy used Table as a sticky container for TD.

Bring TablesNG in sync with legacy, and make
table rows also use table as a sticky container.

Standard not settled yet, discussion at:
w3c/csswg-drafts#5020

Makes 2 layout tests pass:

external/wpt/css/css-position/sticky/position-sticky-table-th-bottom.html
external/wpt/css/css-position/sticky/position-sticky-table-th-top.html

Makes 3 unit tests pass
LayoutBoxModelObjectTest.StickyPositionComplexTableNesting (layout_box_model_object_test.cc:886)
LayoutBoxModelObjectTest.StickyPositionNestedStickyTable (layout_box_model_object_test.cc:818)
LayoutBoxModelObjectTest.StickyPositionTableContainers (layout_box_model_object_test.cc:349)

Bug: 958381
Change-Id: If099efe32a6a23154b53bf96981f7176030cd52c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2542737
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Commit-Queue: Aleks Totic <atotic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#828994}

--

wpt-commits: 1b52cf975cb73d97848b9dee0a1609fe46aebaea
wpt-pr: 26549
@jonjohnjohnson
Copy link

I think the answer should be that the containing block for rows and cells should be their row group, and for row groups it should be the table grid box.

@atotic Are we going to run into compat issues by delaying a possible resolution on @tabatkins suggestion?

@atotic
Copy link
Contributor

atotic commented Jul 20, 2021

I do not think there will be compat issues with tab's proposal.

Chrome has shipped it, and developers are happy.

@tabatkins
Copy link
Member

@atotic Are you saying that Chrome has shipped the "cells/rows stick to their row-container" behavior I suggested above, or just that Chrome has shipped the "all internal table elements stick to the table wrapper" behavior? When I play around with your test from above, I'm seeing cells only stick to the table as a whole.

@atotic
Copy link
Contributor

atotic commented Sep 4, 2021

What I implemented was table wrapper is a container for row-groups/rows/cells.
My comment "I do not think there will be compat issues with tab's proposal." is incorrect.
I got confused, and forgot that that tab's proposal was "row-group is a container for rows/cells".

@jonjohnjohnson
Copy link

@atotic

My comment "I do not think there will be compat issues with tab's proposal." is incorrect.

Do you still think there will be compat issues? If so, should this be discussed/resolved sooner than later to lessen the impending issues? What are the next steps in your opinion?

https://bugs.chromium.org/p/chromium/issues/detail?id=702927

mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
Legacy used Table as a sticky container for TD.

Bring TablesNG in sync with legacy, and make
table rows also use table as a sticky container.

Standard not settled yet, discussion at:
w3c/csswg-drafts#5020

Makes 2 layout tests pass:

external/wpt/css/css-position/sticky/position-sticky-table-th-bottom.html
external/wpt/css/css-position/sticky/position-sticky-table-th-top.html

Makes 3 unit tests pass
LayoutBoxModelObjectTest.StickyPositionComplexTableNesting (layout_box_model_object_test.cc:886)
LayoutBoxModelObjectTest.StickyPositionNestedStickyTable (layout_box_model_object_test.cc:818)
LayoutBoxModelObjectTest.StickyPositionTableContainers (layout_box_model_object_test.cc:349)

Bug: 958381
Change-Id: If099efe32a6a23154b53bf96981f7176030cd52c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2542737
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Commit-Queue: Aleks Totic <atotic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#828994}
GitOrigin-RevId: 957cd8a0523cca212d35089d05bb8dc02a311017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants