Closed Bug 1907289 Opened 4 months ago Closed 4 months ago

Page is slowly scrolling automatically

Categories

(Core :: Layout: Tables, defect)

Firefox 128
defect

Tracking

()

VERIFIED FIXED
130 Branch
Tracking Status
firefox-esr115 --- unaffected
firefox-esr128 --- verified
firefox128 --- verified
firefox129 --- verified
firefox130 --- verified

People

(Reporter: ffsguerreiro, Assigned: emilio)

References

(Regression)

Details

(Keywords: regression)

Attachments

(4 files)

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/126.0.0.0 Safari/537.36

Steps to reproduce:

Actual results:

The page is going up and down, but if you look closely it's going down really slow

Expected results:

it should be a static page

Tried on the previous firefox version and it is not happening this issue (127.02)

Group: firefox-core-security

INFO: Last good revision: 33172968feaa0ef9043a29b0231451411bb57223
INFO: First bad revision: e7f116bf080c7d9ac0fe5fe26a04779a673018ec
INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=33172968feaa0ef9043a29b0231451411bb57223&tochange=e7f116bf080c7d9ac0fe5fe26a04779a673018ec

Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: regression
Regressed by: 1850834

Set release status flags based on info from the regressing bug 1850834

:emilio, since you are the author of the regressor, bug 1850834, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Moving component based on regressor.

Component: Untriaged → Layout
Product: Firefox → Core

The page is doing something odd:

        // This slows down the bind
        $(window).bind('DOMContentLoaded load resize scroll', function (e) {
            var scrollY = window.scrollY;
            if (scrollY >= 0) {
                setTimeout(function () {
                    if (window.scrollY == scrollY)
                        //console.log("calendar scrollY: " + scrollY);
                        bindIt();
                }, 250);
            }
        });

Where bindIt computes adds and removes a table cell in a way that it used to destroy the whole table and now no longer does. That somehow seems to cause scroll anchoring to kick in, and trigger another scroll event, causing the effect here.

They have a helpful debug mode with console logs: https://tradingeconomics.com/calendar?debug

I'll dig to see why scroll anchoring is kicking in.

Adding :root { overflow-anchor: none } "fixes" it.

Submitted a pernosco trace:

Compressing to /tmp/tmp82y1rcc9...
Uploading 1360397165 bytes to s3://pernosco-upload/15EaUmqxv78.tar.zst...
upload: ../../../../../tmp/tmp82y1rcc9 to s3://pernosco-upload/15EaUmqxv78.tar.zst

This is starting to look like a subtle (and probably pre-existing) layout bug, but I gotta dig more.

Attached file Reduced test-case

All tables should look the same. In particular, that shift in "Test 2" shouldn't happen. Before my patch, it was papered because we rebuilt the whole table.

Yeah, so, as suspected, this basically uncovers a bug in the dynamic changes of our border-collapsed tables :'(

cc'ing David who has looked at border-collapsed tables recently, in case he has any idea off-hand of where the bug might lie. I'll debug this regardless of course.

No longer blocks: scroll-anchoring
Component: Layout → Layout: Tables

A potentially better approach could be to call FrameNeedsReflow from
nsBCTableCellFrame::SetBorderWidth... But it seems that might
over-invalidate quite a lot when recalculating borders, since it seems
we reset the relevant ones and then accumulate directly on the frame.

If that seems like a better approach, happy to give that a try. This
should be less risky for performance changes in general tho.

Assignee: nobody → emilio
Status: NEW → ASSIGNED
Flags: needinfo?(emilio)

Comment on attachment 9412498 [details]
Bug 1907289 - Reflow table cell if collapsed borders have changed. r=dshin!

Beta/Release Uplift Approval Request

  • User impact if declined: comment 0
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: comment 0
  • List of other uplifts needed: none
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Relatively low-risk fix for a pre-existing issue that the regressing bug uncovered.
  • String changes made/needed: none
  • Is Android affected?: Yes
Attachment #9412498 - Flags: approval-mozilla-release?
Attachment #9412498 - Flags: approval-mozilla-beta?
Flags: qe-verify+
Pushed by ealvarez@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/a3485259ae0c Reflow table cell if collapsed borders have changed. r=dshin
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/47115 for changes under testing/web-platform/tests
Status: ASSIGNED → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → 130 Branch
Upstream PR merged by moz-wptsync-bot

Comment on attachment 9412498 [details]
Bug 1907289 - Reflow table cell if collapsed borders have changed. r=dshin!

Approved for 129.0b4

Attachment #9412498 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9412498 - Flags: approval-mozilla-esr128?
QA Whiteboard: [qa-triaged]

Reproduced the issue on Firefox 130.0a1 (2024-07-11) on Windows 11 by following the STR from Comment 0.

The issue is fixed on Firefox 129.0b4 (treeherder build) and Firefox 130.0a1 (2024-07-14). Tests were performed on macOS 14.5, Windows 11 and Ubuntu 24.04.

Will verify the fixes on 128 and ESR128 as well when they will be available.

QA Whiteboard: [qa-triaged]
Flags: qe-verify+

Comment on attachment 9412498 [details]
Bug 1907289 - Reflow table cell if collapsed borders have changed. r=dshin!

Approved for 128.1esr.

Attachment #9412498 - Flags: approval-mozilla-esr128? → approval-mozilla-esr128+

Comment on attachment 9412498 [details]
Bug 1907289 - Reflow table cell if collapsed borders have changed. r=dshin!

Approved for 128.0.2

Attachment #9412498 - Flags: approval-mozilla-release? → approval-mozilla-release+

Verified the fix on Firefox 128.0.2 and Firefox 128.1.0esr (treeherder build). Tests were performed on macOS 14.5, Windows 11 and Ubuntu 24.04.

Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: