Make position: sticky work on table parts

NEW
Assigned to

Status

()

Core
Layout
P4
normal
3 years ago
23 hours 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

11 months ago
Blocks: 1267898

Comment 16

3 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

2 months 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

a month 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.

Comment 21

29 days ago
I have been experimenting with using sticky, to use it to implement fixed/floating headers for very large tables on wikipedia. Unfortunately, it seems for Firefox it's indeed not working at all.

Current, partial, implementation of table headers with sticky:
https://en.wikipedia.org/wiki/MediaWiki:Gadget-StickyTableHeaders.css

Example page with 3 tables that could use this behaviour
https://en.wikipedia.org/wiki/List_of_Space_Shuttle_missions?withCSS=MediaWiki:Gadget-StickyTableHeaders.css

Note the 3rd table beneath Flight statistics, has a multirow header, which shows why sticky positioning really needs to work on thead elements and is problematic to have on just th, as the th's will overlap (and with the fluid state of wikipedia, I can't go predict how many row, or how high those rows are, to correct for it.

Note also that with any sticky implementation (including Safari's actually), there is a real problem with borders, which seem impossible to create, in any sane way. The state of sticky positioning is really a shame in my opinion. I appreciate all the potential problems that developers run into, but please also consider that I just want to stop using JS for these kinds of things.

Comment 22

29 days ago
(In reply to Derk-Jan Hartman from comment #21)
> I have been experimenting with using sticky, to use it to implement
> fixed/floating headers for very large tables on wikipedia. Unfortunately, it
> seems for Firefox it's indeed not working at all.
> 
> Current, partial, implementation of table headers with sticky:
> https://en.wikipedia.org/wiki/MediaWiki:Gadget-StickyTableHeaders.css
> 
> Example page with 3 tables that could use this behaviour
> https://en.wikipedia.org/wiki/List_of_Space_Shuttle_missions?withCSS=MediaWiki:Gadget-StickyTableHeaders.css
> 
> Note the 3rd table beneath Flight statistics, has a multirow header, which
> shows why sticky positioning really needs to work on thead elements and is
> problematic to have on just th, as the th's will overlap
 
You are correct that, in case of a multirow header, position: sticky needs be applied to thead instead of individual th's. Unfortunately, support for position: sticky on thead is going to be removed in Chrome 58 because "The effect of ‘position: sticky’ on table related elements is the same as for ‘position: relative’" and "In CSS 2.1, position: relative is not allowed on <thead> and <tr> [...] so until Blink supports CSS 3 positioning we should disallow it" (https://bugs.chromium.org/p/chromium/issues/detail?id=695179). Firefox does support position: relative on thead and tr, but AFAIK is the only (major) browser that does.

> Note also that with any sticky implementation (including Safari's actually),
> there is a real problem with borders, which seem impossible to create, in
> any sane way.

I think the problem is border-collapse: collapse, and it affects both position: sticky and position: relative. There is an illuminating comment by dbaron on position: relative, table cells, background and borders (see bug 35168 comment 32).

I've viewed your Wikipedia page with Firefox 52 and Chrome 57. As to borders disappearing:

- Firefox exhibits a bug that, I think, is bug 688556. That is, I think it currently implements position: sticky on table cells as position: relative and this triggers that bug.

- Chrome seems to follow this rule: a sticky td or th loses a border when it becomes stuck unless border-collapse is separate or that border is not shared with another static cell. This makes sense because, once a cell is moved around by positioning, it can no longer share borders with its static neighbours. So you can't use border-collapse: collapse with position sticky (or relative); you have to style borders using border-spacing, :first- or :last-child etc.

Comment 23

29 days ago
> This makes sense because [..]

Technical sense perhaps. From an end users perspective, i'm just not achieving what I expect to achieve.

> you have to style borders using border-spacing, :first- or :last-child etc.

This assumes that a website author is always fully in control. In wikipedia i can't predict the many types of borders that people want to use, and create special sticky header exceptions for that, nor can I go and make special class definitions for every table variation, there are just too many.
Next to that, I've actually experimented a bit with that strategy before, and the results were less than desirable. Especially with things like col and rowspans in the mix, or sizing constraints, those become really ugly hacks. And that is ignoring that I don't know what color is used in the border that needs to be replicated in a :before :after etc. That I have 4 borders, and not 2 etc. And all those rules also apply to the non-relative positioning (there is no "only apply this when we are actually sticky/relative).

It's a mess :)
Comment hidden (offtopic)
Comment hidden (offtopic)
Comment hidden (offtopic)

Comment 27

28 days ago
Chrome will support this: http://output.jsbin.com/toxiliz,all table header elements will be sticky

Comment 28

28 days ago
Enable position:sticky for <thead> and <tr> elements

https://codereview.chromium.org/2786743002/

Comment 29

23 hours ago
(In reply to Francesco from comment #22)

> - Chrome seems to follow this rule: a sticky td or th loses a border when it
> becomes stuck unless border-collapse is separate or that border is not
> shared with another static cell.

I correct myself: Chrome does *not* preserve any sticky table cell's border with border-collapse: collapse. It moves the cell and its background out of its collapsed borders which remain static. That is, it behaves the same as it (and Firefox) behave for position: relative. Sorry for the confusion.

As to comment 27, support for position: sticky on <thead> and <tr> has been removed in Chrome 58.
You need to log in before you can comment on or make changes to this bug.