Closed Bug 1873301 Opened 2 years ago Closed 1 year ago

Buttons don't support scrollbars (overflow:scroll/auto) in Firefox

Categories

(Core :: Layout: Form Controls, defect)

defect

Tracking

()

VERIFIED FIXED
142 Branch
Tracking Status
firefox142 --- verified
firefox143 --- verified

People

(Reporter: dholbert, Assigned: emilio)

References

Details

(Keywords: perf-alert, webcompat:platform-bug)

Attachments

(2 files, 1 external file, 1 obsolete file)

I'm not sure there's a huge demand for this, but I noticed it as an interop difference between us and Chromium/WebKit in response to IanK's paranthetical in https://github.com/w3c/csswg-drafts/issues/7714#issuecomment-1877870277

Apparently <button style="overflow:scroll"> (and "auto") can successfully add scrollbars in Chromium and Safari, but not in Firefox.

STR:

  1. Load testcase.
  2. Try to scroll the button to find the text "YouFoundMe" on the right side or the bottom side of the button's content.

ACTUAL RESULTS:
No scrollbars; no way to reach the "YouFoundMe" text.

EXPECTED RESULTS:
Maybe there should be scrollbars so that the overflowing text can be found (per the overflow styling).

Attached file testcase 1 —

As you can see in the screenshot, Chromium/WebKit show scrollbars.

And also, as you might expect/hope: clicking the scrollbars does not seem to actually click the button (at least: if I add an onclick handler for the button, that handler doesn't fire when I click or click-and-drag the scrollbar).

I have a branch that makes button a regular block with align-content: center which I believe would fix this, but was still failing some tests IIRC.

Blocks: 1977022

This removes the implementation of nsHTMLButtonControlFrame, instead
reusing nsBlockFrame + align-content (or the relevant display type for
flex / grid / columns / etc).

This matches what Blink does and, for the most part, it is trivial.
It requires moving some special cases around, and override inline flow
display, tho so that buttons don't suddenly start fragmenting in the
inline axis.

It adds a tiny fix for a writing-mode mismatch in
nsBlockFrame::AlignContent(), covered by
layout/reftests/writing-mode/1138356-1-button-contents-alignment.html

Assignee: nobody → emilio
Status: NEW → ASSIGNED
Depends on: 1977267
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/53765 for changes under testing/web-platform/tests
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → 142 Branch
Upstream PR merged by moz-wptsync-bot
Regressions: 1977669
Regressions: 1977839
Regressions: 1977647
Blocks: 1978178

This is not doing anything now: out of flows and root element (the other
cases where we set the adjusted display) already stop text decoration
propagation.

I wrote this code in bug 1873301 because my initial approach adjusted
(at style time) display: inline on a button to display: inline-block. That approach did almost work, except it somewhat changes
behavior for thins like display: table-cell on a button or so, so I
rejiggered stuff to happen at frame construction time instead, making
this special-case unnecessary.

Comment on attachment 9501642 [details]
Bug 1873301 - Remove unnecessary special-case for text decoration skipping. r=#layout

Revision D257912 was moved to bug 1978178. Setting attachment 9501642 [details] to obsolete.

Attachment #9501642 - Attachment is obsolete: true
Regressions: 1978487
QA Whiteboard: [qa-triage-done-c143/b142][qa-ver-needed-c143/b142]
Flags: qe-verify+

Issue is reproducible on a 2025-07-13 Firefox Nightly build on Windows 10.
Verified as fixed on Firefox Nightly 143.0a1 and Firefox 142.0b2 on Windows 10, Ubuntu 22, macOS 15.

Status: RESOLVED → VERIFIED
QA Whiteboard: [qa-triage-done-c143/b142][qa-ver-needed-c143/b142] → [qa-triage-done-c143/b142][qa-ver-done-c143/b142]
Flags: qe-verify+

(In reply to Pulsebot from comment #9)

Pushed by sstanca@mozilla.com:
https://github.com/mozilla-firefox/firefox/commit/47335a90a52d
https://hg.mozilla.org/integration/autoland/rev/74a2ad907c9b
Suppress scrollbar that used to be out of the viewport.

Perfherder has detected a devtools performance change from push 197645ffd8065cb1c8bc1137f804091695494284.

If you have any questions, please reach out to a performance sheriff. Alternatively, you can find help on Slack by joining #perf-help, and on Matrix you can find help by joining #perftest.

Improvements:

Ratio Test Platform Options Absolute values (old vs new)
4% damp custom.inspector.manyrules.deselectnode linux1804-64-shippable-qr e10s fission stylo webrender 249.11 -> 239.62
3% damp custom.inspector.manyrules.deselectnode windows11-64-24h2-shippable e10s fission stylo webrender 125.24 -> 121.33

Details of the alert can be found in the alert summary, including links to graphs and comparisons for each of the affected tests.

If you need the profiling jobs you can trigger them yourself from treeherder job view or ask a performance sheriff to do that for you.

You can run all of these tests on try with ./mach try perf --alert 45981

The following documentation link provides more information about this command.

Keywords: perf-alert
Regressions: 1984884
Regressions: 1985089
Duplicate of this bug: 1712594
Duplicate of this bug: 1394631
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: