1934960 - Unexpected overflow in auto-sized <pre> elements.

Status: RESOLVED FIXED

(Core :: Layout: Scrolling and Overflow, defect)

Description

[Emilio Cobos Álvarez [:emilio]]

Attachment: Test-case

[Tracking Requested - why for this release]: Regression on very common functionality.

I've seen this in phabricator emails, but also in some code blocks like https://devblogs.microsoft.com/oldnewthing/20100312-01/?p=14623. See test-case.

Comment 1

[Emilio Cobos Álvarez [:emilio]]

The smaller line-height seems like a necessary part of this.

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

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

Comment 3

[David Shin[:dshin]]

Attachment: Exaggerated Testcase

Exaggerating a bit with zero line heights.
Curiously, Chrome does not generate any scroll for these.

Comment 4

[Donal Meehan [:dmeehan]]

Moving tracking request from 133, since the regressor landed in 134.
Tracking for 134/135.

Comment 5

[Daniel Holbert [:dholbert]]

Attachment: testcase 3 (large font, even larger padding)

The previous testcases didn't show scrollbars for me for whatever reason (on ubuntu with "always show scrollbars"); possibly because at least in the second testcase, the scrollable areas are shorter than our limits on where we'll draw scrollbars (for my platform at least).

Here's a testcase where I've increased the sizes a bit which I think make this a bit more reliably reproducible.

In this testcase, Firefox shows a vertical scrollbar for all three elements, whereas Chrome only shows a scrollbar for the first one.

Comment 6

[Daniel Holbert [:dholbert]]

Attachment: testcase 4 (large font, not quite as large padding)

Here's a testcase (variant of the previous one with adjusted sizes) where Chrome and Firefox both show scrollbars on all three divs; but Firefox's scrollbars for the 2nd and 3rd boxes let you scroll the text further out of view than Chrome does.

Comment 7

[Daniel Holbert [:dholbert]]

Firefox release 133.0 matches Chrome on testcases 3 and 4, fwiw, in terms of how far it lets me scroll. (And Firefox release matches Chrome on testcases 1 and 2 in that mousewheel-scrolling has no effect on the scrollable areas in those testcases, whereas mousewheel-scrolling does scroll the areas there for me in Nightly.)

Comment 8

[Daniel Holbert [:dholbert]]

Attachment: testcase 5 (with margin-top and/or margin-bottom on the inline boxes)

Here's a testcase with margins on the inline boxes (spans) in the scrollable area.

It seems that specifically the spans that have margin-bottom end up inflating the scrollable height of their scroll frame -- specifically the last two boxes in each row here. In case this isn't easy to see from the scrollbar thumb-sizes, I added some logging at the end of the testcase to show the scrollHeight values.

In Firefox 133 and Chrome, the scrollHeight is consistent for each row (each grouping of 4 elements in the logging), and we essentially agree with Chrome on the heights themselves, too.

In Firefox Nightly 135, the scrollHeight is larger for the latter 2 elements in each group (the ones with margin-bottom via the mb class).

Comment 9

[Daniel Holbert [:dholbert]]

Attachment: testcase 5 (with margin-top and/or margin-bottom on the inline boxes)

(sorry, I uploaded the wrong file for testcase 5; here's the one I meant to attach)

Comment 10

[Daniel Holbert [:dholbert]]

Attachment: testcase 6 (simpler case with `margin-bottom`)

Here's a testcase just focusing on margin-bottom on an inline element, without any padding, and also with a second line below the element that has margin.

In Firefox Nightly, the margin-bottom creates scrollable overflow and a scrollbar, despite not actually moving the next line down. (I think block-axis margins aren't supposed to have any effect on display:inline elements, at least in this case, and perhaps in general?)

Comment 11

[David Shin[:dshin]]

Attachment: Bug 1934960: Line participant frames contribute to padding-inflated scrollable overflow only up to line bounds. r=#layout

Comment 12

[David Shin[:dshin]]

Attachment: Bug 1934960: Remove workaround for avoiding block-direction scroll in single-line input elements. r=#layout

... Unfortunately, the <textarea> hack remains, since the previous patch
only affects padding inflation in block direction.

Depends on D231028

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6b3d46295057
Line participant frames contribute to padding-inflated scrollable overflow only up to line bounds. r=layout-reviewers,emilio
https://hg.mozilla.org/integration/autoland/rev/b629ec5f9f85
Remove workaround for avoiding block-direction scroll in single-line input elements. r=layout-reviewers,emilio
https://hg.mozilla.org/integration/autoland/rev/04fef5ae1a55
apply code formatting via Lando

Comment 14

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/49534 for changes under testing/web-platform/tests

Comment 15

[amarc]

Backed out for causing wpt fails @ scrollable-overflow-padding-inline.html

• Backout link
• Push with failures
• Failure Log
• Failure line:

TEST-UNEXPECTED-FAIL | /css/css-overflow/scrollable-overflow-padding-inline.html | .scroller 1 - assert_equals:

Comment 16

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR was closed without merging

Comment 17

[David Shin[:dshin]]

Right, viewport meta tag for android tests.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/de4fe401901e
Line participant frames contribute to padding-inflated scrollable overflow only up to line bounds. r=layout-reviewers,emilio
https://hg.mozilla.org/integration/autoland/rev/d0e3f691f780
Remove workaround for avoiding block-direction scroll in single-line input elements. r=layout-reviewers,emilio
https://hg.mozilla.org/integration/autoland/rev/e8e6e0ddbb41
apply code formatting via Lando

Comment 19

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/de4fe401901e
https://hg.mozilla.org/mozilla-central/rev/d0e3f691f780
https://hg.mozilla.org/mozilla-central/rev/e8e6e0ddbb41

Comment 20

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 21

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:dshin, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox134 to wontfix.

For more information, please visit BugBot documentation.

Comment 22

[David Shin[:dshin]]

Comment on attachment 9441541 [details]
Bug 1934960: Line participant frames contribute to padding-inflated scrollable overflow only up to line bounds. r=#layout

Beta/Release Uplift Approval Request

• User impact if declined/Reason for urgency: Extra unncessary scrollbars (Confirmed multiple websites-in-the-wild, in comment 0)
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce: N/A
• Verified through mozregression --launch 2024-12-05 with comment 0 testcase
• Covered by added WPT /css/css-overflow/scrollable-overflow-padding-inline.html
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): May lead to pre-Bug 1768921 - which is to say, inline scroll-end doesn't have padding. Difference would be purely visual.
• String changes made/needed:
• Is Android affected?: Yes

Comment 23

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/8b127e5d2c59