Make position: sticky work on table parts

VERIFIED FIXED in Firefox 59

Status

()

defect
P4
normal
VERIFIED FIXED
5 years ago
3 months ago

People

(Reporter: seth, Assigned: emilio)

Tracking

(Depends on 1 bug, Blocks 1 bug, {dev-doc-complete})

Trunk
mozilla59
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox59 verified)

Details

Attachments

(3 attachments, 1 obsolete attachment)

Reporter

Description

5 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).
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).
Priority: -- → P4
Note that nsFrame::Init has code to disable this, where it checks !disp->IsInnerTableStyle().
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.)
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)
Attachment #8542609 - Attachment is obsolete: true
Attachment #8542609 - Flags: review?(dbaron)
Attachment #8542610 - Flags: review?(dbaron)
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?
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: 1267898

Comment 16

2 years 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/
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 years 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

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

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

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

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

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

Comment 28

2 years ago
Enable position:sticky for <thead> and <tr> elements

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

Comment 29

2 years 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.

Comment 30

2 years ago
(In reply to Francesco from comment #29)
> (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.

According to https://bugs.chromium.org/p/chromium/issues/detail?id=702927
<thead> and <tr> arent, but <th> is working. Just tested it.
 
This is probably what most developers are asking for.

Comment 31

2 years ago
Bug 929484, a blocker of this bug, has just been fixed. It will be interesting to know if that does something about the issues of comment 11 and comment 15 above.

Comment 32

2 years ago
https://css-tricks.com/idea-simple-responsive-spreadsheet/ << Doesn't work in FireFox because of this bug.
Assignee

Updated

2 years ago
See Also: → 1421660
(In reply to Jamie Katz from comment #32)
> https://css-tricks.com/idea-simple-responsive-spreadsheet/ << Doesn't work
> in FireFox because of this bug.

I just did a quick try with a rebased patch I see no issues, so I may try to tidy it and land it (it's running through the tryserver now).

That demo still doesn't work... that's because of bug 1421660.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 38

2 years ago
> This matches Blink's behavior.
Be aware that blinks behaviour changed 10 hours ago when this issue was marked as fixed:
https://bugs.chromium.org/p/chromium/issues/detail?id=702927
(In reply to m.kurz from comment #38)
> > This matches Blink's behavior.
> Be aware that blinks behaviour changed 10 hours ago when this issue was
> marked as fixed:
> https://bugs.chromium.org/p/chromium/issues/detail?id=702927

Just tested it, we also pass that test (kinda unsurprisingly, since we already supported relative positioning on those), but thanks a lot for mentioning it! :)

Comment 40

2 years ago
(In reply to Emilio Cobos Álvarez [:emilio] from comment #39)
> Just tested it, we also pass that test (kinda unsurprisingly, since we
> already supported relative positioning on those), but thanks a lot for
> mentioning it! :)

So <thead style=position:sticky> works? Great, that’s my main usecase prevented by this bug.

(In reply to John Rockaxe from comment #30)
> According to https://bugs.chromium.org/p/chromium/issues/detail?id=702927
> <thead> and <tr> arent, but <th> is working. Just tested it.
>  
> This is probably what most developers are asking for.

What? No! sticky <th> are pretty useless. why would you want a single header cell to stick?

<thead> and <tr> are the most useful table parts to make sticky.
(In reply to flying sheep from comment #40)
> (In reply to Emilio Cobos Álvarez [:emilio] from comment #39)
> > Just tested it, we also pass that test (kinda unsurprisingly, since we
> > already supported relative positioning on those), but thanks a lot for
> > mentioning it! :)
> 
> So <thead style=position:sticky> works? Great, that’s my main usecase
> prevented by this bug.
> 
> (In reply to John Rockaxe from comment #30)
> > According to https://bugs.chromium.org/p/chromium/issues/detail?id=702927
> > <thead> and <tr> arent, but <th> is working. Just tested it.
> >  
> > This is probably what most developers are asking for.
> 
> What? No! sticky <th> are pretty useless. why would you want a single header
> cell to stick?
> 
> <thead> and <tr> are the most useful table parts to make sticky.

They all work with these patches. I thought <th> would be useless too, but apparently people use it (see comment 32)...

Comment 42

2 years ago
> What? No! sticky <th> are pretty useless. why would you want a single header cell to stick?

We actively use sticky <th> to work around the disabled <thead> support on current Chrome stables to provide sticky table headers. I don't think anyone would want to apply this to a single <th>. Applying to all works for sticky headers as one expects.

Comment 43

2 years ago
(In reply to flying sheep from comment #40)
> (In reply to Emilio Cobos Álvarez [:emilio] from comment #39)
> > Just tested it, we also pass that test (kinda unsurprisingly, since we
> > already supported relative positioning on those), but thanks a lot for
> > mentioning it! :)
> 
> So <thead style=position:sticky> works? Great, that’s my main usecase
> prevented by this bug.
> 
> (In reply to John Rockaxe from comment #30)
> > According to https://bugs.chromium.org/p/chromium/issues/detail?id=702927
> > <thead> and <tr> arent, but <th> is working. Just tested it.
> >  
> > This is probably what most developers are asking for.
> 
> What? No! sticky <th> are pretty useless. why would you want a single header
> cell to stick?
> 
> <thead> and <tr> are the most useful table parts to make sticky.

What comment 42 said, 
we are waiting for a solution on thead, 
meanwhile th(on all columns) is the only workaround(on chrome at least) on css.

Comment 44

2 years ago
Regarding #40 

> why would you want a single header cell to stick?


In my use case, we're trying to sticky the :first-child of the th, and tr's to the left. So the end result would be that the first column in the table is always sticky, and the rest of the table can be panned right or left. 

This is a pain-in-the-butt bug IMO. Please fix it :)

Comment 45

a year ago
mozreview-review
Comment on attachment 8933958 [details]
Bug 975644: Add a hack to skip table row groups for sticky positioning.

https://reviewboard.mozilla.org/r/204894/#review214602

r=me, but please mention this code explicitly in bug 1421660?
Attachment #8933958 - Flags: review?(bzbarsky) → review+

Comment 46

a year ago
mozreview-review
Comment on attachment 8933959 [details]
Bug 975644: Enable position sticky in table parts.

https://reviewboard.mozilla.org/r/204896/#review214604
Attachment #8933959 - Flags: review?(bzbarsky) → review+

Comment 47

a year ago
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/bdafa059aa94
Add a hack to skip table row groups for sticky positioning. r=bz
https://hg.mozilla.org/integration/autoland/rev/d473c27fc7f8
Enable position sticky in table parts. r=bz

Comment 48

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/bdafa059aa94
https://hg.mozilla.org/mozilla-central/rev/d473c27fc7f8
Status: NEW → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
I've updated the documentation of https://developer.mozilla.org/en-US/docs/Web/CSS/position#sticky to include table-related elements. The compatiblity table still needs to be updated to reflect the changes.

Sebastian
Keywords: dev-doc-needed
I have documented this on MDN by:

* Adding a note to the Fx59 rel notes:

https://developer.mozilla.org/en-US/Firefox/Releases/59#CSS

* Updating the browser compat information:

https://github.com/mdn/browser-compat-data/pull/872

Let me know if this looks OK. Thanks!
Flags: needinfo?(corey)
Looks fine to me (I'm not up to date on other browsers' support) but probably better for Emilio to confirm.
Assignee: corey → emilio
Flags: needinfo?(corey) → needinfo?(emilio)
Just confirmed this works for our use case (sticky THs on an innner scrolling table) on Nightly 59.0a1 (2018-01-21).

This is amazing news and will finally make sticky tables with flexible table layouts a piece of cake. Thank you all for your efforts!
Looks fine to me too. Maybe worth pointing out on the release notes that "Table elements" doesn't mean just <table>, but all the table parts?

Other than that seems fine, thanks Chris!
Flags: needinfo?(emilio)
Status: RESOLVED → VERIFIED
(In reply to Emilio Cobos Álvarez [:emilio] from comment #53)
> Looks fine to me too. Maybe worth pointing out on the release notes that
> "Table elements" doesn't mean just <table>, but all the table parts?
> 
> Other than that seems fine, thanks Chris!

Good point. I've updated the note to make this clearer.

Comment 55

a year ago
(In reply to Francesco from comment #22)
> Firefox does support position: relative on thead and tr, but AFAIK is the
> only (major) browser that does.

<tr> seems to be working in Safari, but not Firefox:
https://codepen.io/anon/pen/mXwpyw

Comment 56

a year ago
Oops, I missed that that comment was about relative (not sticky)
For what it's worth, the link in comment 55 should work in Firefox 59 (and definitely works in nightly).

Comment 58

a year ago
Thanks for the release of "position: sticky" support with Firefox 59! I’ve been following the issue for some time now.

I think there’s a bug related to sticky positioned elements with parents defining "overflow" other than "visible", though.

Please have a look at:
- https://bugzilla.mozilla.org/show_bug.cgi?id=1329203
- https://github.com/w3c/csswg-drafts/issues/865
That's an open spec issue, right?  As in, the behavior for that case needs to actually be defined in the spec so we can implement it...

Comment 60

a year ago
As mentioned in https://github.com/w3c/csswg-drafts/issues/865#issuecomment-324392974 the current spec (https://www.w3.org/TR/css-position-3/#sticky-pos) does not describe that "position: sticky" should not work inside a container defining "overflow" other than "visible". So I thought the current behavior in combination with "overflow: [hidden|scroll|auto]" might not be correct.
Please stick to one issue per bug.  Having a conversation about one bug in a fixed bug on a different topic just confuses things and makes everything less likely to get fixed.
Depends on: 1472602

Comment 62

3 months ago

The sticky in tables still work incorectly on table-row elements...
https://jsfiddle.net/sj5xb39c/

Why does the sticky layer disappeared after 30 seconds??? What part of spec define this behaviour?

Comment 63

3 months ago

Looks like not fixed. Bad status. Please change status of this bug. Please see above.

Flags: needinfo?(sebastianzartner)
Assignee

Comment 64

3 months ago

(In reply to Risa A from comment #63)

Looks like not fixed. Bad status. Please change status of this bug. Please see above.

Please file a new bug blocking this one instead.

Flags: needinfo?(sebastianzartner)

Updated

3 months ago
Depends on: 1528957
You need to log in before you can comment on or make changes to this bug.