Closed Bug 975644 Opened 10 years ago Closed 6 years ago

Make position: sticky work on table parts

Categories

(Core :: Layout, defect, P4)

Product:
Core
Component:
Layout
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla59
Tracking Flags:
Tracking Status
firefox59 --- verified

People

(Reporter: seth, Assigned: emilio)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

(Keywords: dev-doc-complete)

Attachments

(3 files, 1 obsolete file)

Allow sticky positioning of internal table elements
6.90 KB, patch
dbaron
: review+
Details | Diff | Splinter Review
Bug 975644: Add a hack to skip table row groups for sticky positioning.
59 bytes, text/x-review-board-request
bzbarsky
: review+
Details
Bug 975644: Enable position sticky in table parts.
59 bytes, text/x-review-board-request
bzbarsky
: review+
Details 
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.
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
(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)
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
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.
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.
(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.
> 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 :)
Chrome will support this: http://output.jsbin.com/toxiliz,all table header elements will be sticky
Enable position:sticky for <thead> and <tr> elements

https://codereview.chromium.org/2786743002/
(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.
(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.
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.
https://css-tricks.com/idea-simple-responsive-spreadsheet/ << Doesn't work in FireFox because of this bug.
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.
> 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! :)
(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)...
> 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.
(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.
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 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 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+
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
https://hg.mozilla.org/mozilla-central/rev/bdafa059aa94
https://hg.mozilla.org/mozilla-central/rev/d473c27fc7f8
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox59: --- → fixed
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)
Keywords: dev-doc-needed → dev-doc-complete
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
status-firefox59: fixed → 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.
(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
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).
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...
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

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?

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

Flags: needinfo?(sebastianzartner)

(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)
Depends on: 1528957
No longer depends on: 1528957
Regressions: 1528957
Depends on: 1727119
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: