The default bug view has changed. See this FAQ.

Make position: sticky work on table parts

NEW
Assigned to

Status

()

Core
Layout
P4
normal
3 years ago
9 days ago

People

(Reporter: seth, Assigned: coyotebush)

Tracking

(Depends on: 1 bug, Blocks: 3 bugs)

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

3 years ago
People want to use position: sticky with table parts, but we can't implement it until table parts support relative positioning (bug 35168) and we create display items for individual table parts (bug 929484).
(Assignee)

Comment 1

3 years ago
All position: sticky elements should create their own display items already, and position: sticky creates a stacking context. But I admit I haven't looked at the behavior of sticky table parts with just the patches from bug 35168 (+ partial revert of bug 925259).

Updated

3 years ago
Priority: -- → P4
Note that nsFrame::Init has code to disable this, where it checks !disp->IsInnerTableStyle().
(Assignee)

Comment 3

3 years ago
That's the code from bug 925259 that I was referring to. In bug 35168 comment 52, I suggested that removing that code might be the only change necessary, but according to Seth it wasn't.
Summary: Make position: sticky work on tables parts → Make position: sticky work on table parts
Another change that will certainly need to be made (noticed this while reviewing bug 35168) is replacing nsTableRowGroupFrame::SlideChild with MovePositionBy.
Also, in my review of bug 35168 (to be posted there shortly), I'm suggesting making it so that this bug will also require changing RestyleManager::RecomputePosition.
(In reply to David Baron [:dbaron] (needinfo? me) (UTC-7) from comment #4)
> Another change that will certainly need to be made (noticed this while
> reviewing bug 35168) is replacing nsTableRowGroupFrame::SlideChild with
> MovePositionBy.

Er, actually, bug 35168 fixes it up correctly, I guess, so never mind.  Might still be good to do, though.
Duplicate of this bug: 1047273
bug 1115999 also fixed an issue that's needed here.  (I'm not sure if there's much else.)
Depends on: 1115999
(Assignee)

Comment 9

2 years ago
Created attachment 8542609 [details] [diff] [review]
Allow sticky positioning of internal table elements.

Removing those checks seems to suffice, yes.

(Note that this leaves only one caller of nsStyleDisplay::IsInnerTableStyle, the one in ContentHelper.cpp added in bug 1005937. Maybe that one should just be inlined.)

Confirmed locally that the updated test fails without the code changes, and here's a try run:
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=d3d1a7297a71
Assignee: nobody → corey
Attachment #8542609 - Flags: review?(dbaron)
(Assignee)

Comment 10

2 years ago
Created attachment 8542610 [details] [diff] [review]
Allow sticky positioning of internal table elements
(Assignee)

Updated

2 years ago
Attachment #8542609 - Attachment is obsolete: true
Attachment #8542609 - Flags: review?(dbaron)
(Assignee)

Updated

2 years ago
Attachment #8542610 - Flags: review?(dbaron)
(Assignee)

Comment 11

2 years ago
In further experiments (on Linux), I'm seeing painting glitches, and even some positioning problems reminiscent of bug 915474, so maybe bug 929484 actually is needed for this to work properly.
Comment on attachment 8542610 [details] [diff] [review]
Allow sticky positioning of internal table elements

The code looks fine.

The test looks like it's not testing all that many cases, but maybe it's enough.  (I didn't look that closely.  It seems worth checking that all the different types of table parts stick, which probably involves 3 cases for each.  Although maybe that's overkill since all the code is generic and it ought to just work.)

That said, it sounds like if there are still problems, this needs to wait to land until that other work is complete.

r=dbaron, although it shouldn't land until the necessary dependencies are fixed
Attachment #8542610 - Flags: review?(dbaron) → review+
This could be very useful for ECMAScript compat tables — https://kangax.github.io/compat-table/es6/ — and Safari is the only one that supports it (Chrome is working on general "position:sticky" too; not sure about tables support).

David, anything preventing this from landing?
comment 11, I think
(Assignee)

Comment 15

2 years ago
Still looks not quite right when applying the patch on current m-c, with a fresh profile.
More precisely, if I

 1. load (arbitrarily chosen)
    https://en.wikipedia.org/w/index.php?title=List_of_Danish_films_of_the_2000s&oldid=557566178
 2. add an inline style via devtools, e.g. in console
    document.querySelector('.wikitable tr:first-child').style = "position: sticky; top: 0"
 3. scroll up and down a lot

the header row occasionally disappears for a moment.
Blocks: 1191995

Updated

10 months ago
Blocks: 1267898

Comment 16

2 months ago
(In reply to Juriy "kangax" Zaytsev from comment #13)
> (Chrome is working on general "position:sticky" too; not sure
> about tables support).
> 

position: sticky is about to land in Chrome Stable[1]  and it does support sticky thead in my latest experiments[2], which at the time of writing still requires canary.

Since position: sticky would be an optimal replacement for floathead and other CSS-based workaround for sticky table headers, reconsidering a solution if at all possible would make lots of developers, myself included, very happy.


[1] https://developers.google.com/web/updates/2016/12/position-sticky
[2] https://jsfiddle.net/zoctn6hd/
Blocks: 53702
Is somebody going to work on this?
Flags: needinfo?(dbaron)
Perhaps; it's on a list I have that I need to share with other layout folks of things that should probably get worked on, although it's not at the top of the priority list these days.
Flags: needinfo?(dbaron)

Comment 19

14 days ago
Hi David,

thanks a lot for all the effort you and your team put into FF.

This feature would make the development of modern web applications much easier. We would be very very grateful if you could have a look at this.

Best
Niels

Comment 20

9 days ago
sticky is "in development" now for MS Edge:
https://github.com/MicrosoftEdge/Status/pull/507

And I hope they make it work for table headers as well - which would make Firefox the last browser which would lack this ability.
You need to log in before you can comment on or make changes to this bug.