1528957 - The sticky position works wrongly for table-row elements

Status: RESOLVED FIXED

(Core :: Web Painting, defect, P3)

Description

[Risa A]

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:60.0) Gecko/20100101 Firefox/60.0

Steps to reproduce:

he 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?

Versions: Firefox for windows: 60.5.1 and 65 affected.

Actual results:

The sticky layer disappeared

Comment 1

[Risa A]

Workaround for Firefox 60.5.1:
.header
{
display:table-row-group;
}

Comment 2

[Boris Zbarsky [:bzbarsky]]

Seems like a painting bug. Switching window visibility a bit (changing to other windows that cover up this one, then changing back) makes the sticky row paint again.

Comment 3

[Risa A]

Next test (without workaround). table-caption also affected but other effect.

https://jsfiddle.net/f6r2dptu/3/

Comment 4

[Matt Woodrow (:mattwoodrow)]

I timed it, and the sticky row disappears after 15 seconds, which is the same as apz.displayport_expiry_ms.

Looks like we remove the displayport due to inactivity, and that breaks painting of the sticky content.

Comment 5

[Matt Woodrow (:mattwoodrow)]

It looks like nsTableRowGroupFrame::BuildDisplayList, DisplayRows, and nsTableRowGroupFrame::GetFirstRowContaining are all using the 'normal' position of the row to decide if we should build display items.

In the broken case, the sticky content is on screen, but it's 'normal' (without position:relative/sticky applied) is offscreen. So we're deciding not to display it.

I think this code is from before we supported position:relative properly, and should just be changed to use the visual overflow rect like we do normally.

Comment 6

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1528957 - Use the frame's rect to advance nsTableRowGroupFrame's cursor, since that's what the max overflow values are computed relative to. r?emilio

Depends on D29447

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a0195a554c70
Use the frame's rect to advance nsTableRowGroupFrame's cursor, since that's what the max overflow values are computed relative to. r=emilio

Comment 9

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/a0195a554c70

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Is this something we should consider backporting to Beta ahead of the next ESR?

Comment 11

[Matt Woodrow (:mattwoodrow)]

I don't think we need to, it's a long standing issue, and this patch is part of a larger table painting rewrite (that I don't want to risk uplifting).

Comment 12

[Timea Cernea [:tbabos][inactive]]

Verified - Fixed on latest Nightly 69.0a1 (2019-06-12) (64-bit) on Windows 10, Mac OS 10.13 and Ubuntu 18.04.
The sticky layer remains displayed.

Comment 13

[Mark Straver]

I'm afraid this isn't fixed. I just tested this on Nightly 72.0a1 (2019-11-30) and the sticky cell still disappears.
Fiddle: https://jsfiddle.net/s9xq5v6n/
With the note that after the patch in this bug it only disappears if you move the mouse cursor over the actual "test" column, not when over other parts of the scrollable fiddle frame. If you keep the mouse outside of it, the sticky element stays visible. The APZ delay still applies for the disappearance; disabling APZ makes it happen a lot more quickly.

Comment 14

[Risa A]

I confirm, the NEXT regression was made. Please set Priority P2. The Bug is NOT FIXED and NOT VERIFIED now.

I'm affraid that Mozilla needs stronker programmers and stronker debugging procedures and mooore TEST-CASES! Please make an automated test-case to this bug.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

Redirecting the needinfo to the bug assignee (though I don't think Matt is actively working on Layout these days, so we may need to redirect it again). Confirmed that the tweaked STR still reproduce (including in Fx69, so this didn't re-regress).

Comment 16

[Risa A]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #15)

Redirecting the needinfo to the bug assignee (though I don't think Matt is actively working on Layout these days, so we may need to redirect it again). Confirmed that the tweaked STR still reproduce (including in Fx69, so this didn't re-regress).

Thus: flagged as Verified and Fixed without real verification and fix? This is very rresponsible behaviour.

Comment 17

[Boris Zbarsky [:bzbarsky]]

Risa A, the originally listed steps to reproduce were in fact verified as fixed. I strongly urge you to read https://bugzilla.mozilla.org/page.cgi?id=etiquette.html and try to follow it.

Comment 18

[Matt Woodrow (:mattwoodrow)]

I filed bug 1607096 for the variant of this issue that is still broken.