Closed Bug 1508441 Opened 6 years ago Closed 6 years ago

[css-contain] contain:size on button shouldn't suppress baseline alignment (but contain:layout should)

Categories

(Core :: Layout, defect, P3)

Product:
Core
Component:
Layout
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Tracking Flags:
Tracking Status
firefox65 --- disabled
firefox66 --- disabled
firefox67 --- disabled
firefox67.0.1 --- disabled
firefox68 --- fixed

People

(Reporter: dholbert, Assigned: dholbert)

References

Details

Attachments

(2 files)

test.html
(deleted), text/html
Details
Bug 1508441 part 1: Adjust button reflow to suppress baseline for contain:layout, instead of for contain:size. r?TYLin
(deleted), text/x-phabricator-request
Details
Attached file test.html (deleted) —
Looks like we missed a spot in bug 1491235 -- "contain:size" still seems to suppress baseline alignment on buttons, but it should not. (but contain:layout should) STR: 1. Load attached testcase. EXPECTED RESULTS: - The first two buttons should have the same vertical alignment (honoring the text's baseline). - The last 3 buttons should have the same vertical alignment (not honoring the text's baseline). (Note: TYLin noticed that this might be a problem in bug 1507663 comment 2, via a test that still expects the old now-incorrect behavior.)
"contain:layout" doesn't currently suppress baseline alignment on a few other form controls, too: - <input> type=button, submit, text, number, date, time - <textarea> Let's fix that here as well. I'm adding a reftest in bug 1507663 which covers these & which I'll annotate as 'fails' with a reference to this bug.
The new WPT test in bug 1514838, "css/css-contain/contain-layout-baseline-005.html", also exercises all of these & reveals a few more things where we aren't yet making "contain:layout" suppress baseline: - inline-grid - inline-table

The original issue (for <button> and button-flavored widgets) turns out to be pretty trivial -- we just need to do s/IsContainSize/IsContainLayout/ here in the ascent-computation portion of nsHTMLButtonControlFrame::ReflowButtonContents():
https://searchfox.org/mozilla-central/rev/75294521381b331f821aad3d6b60636844080ee2/layout/forms/nsHTMLButtonControlFrame.cpp#314-320

Assignee: nobody → dholbert
Status: NEW → ASSIGNED

(So far there's just "part 1", but I'll be posting a "part 2" here later on, to address the issues/test-failures noted in comment 1 and 2.)

(In reply to Daniel Holbert [:dholbert] from comment #5)

(So far there's just "part 1", but I'll be posting a "part 2" here later on, to address the issues/test-failures noted in comment 1 and 2.)

Haven't gotten to this yet and I've got a bit of a review backlog now -- so I'm going to go ahead and land Part 1 now, and I'll post Part 2 in a few days, either here or in a followup bug. (Since we've got open test-failures with this bug number in an annotation-comment, I'll tentatively still plan on doing it in this bug.) (I think this is possible via Lando, though worst case I can just land Part 2 on Inbound.)

Keywords: leave-open
Pushed by dholbert@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/7e12e84c8716 part 1: Adjust button reflow to suppress baseline for contain:layout, instead of for contain:size. r=TYLin
Blocks: 1552287

I spun off bug 1552287 for the remaining work here. Let's close this bug out, since the original testcase has been addressed.

Earlier releases aren't affected because containment is preffed off there, after the "early-beta" period (via the guard in bug 1532471).

Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox65: affected → disabled
status-firefox66: --- → disabled
status-firefox67: --- → disabled
status-firefox67.0.1: --- → disabled
status-firefox68: --- → fixed
Resolution: --- → FIXED
Pushed by dholbert@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/73c897cf5ec0 followup: adjust reftest failure annotation to point to an updated bug number instead of this closed bug. (no review, test-manifest-only, DONTBUILD)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: