Closed Bug 1486086 Opened Last year Closed Last year

In vertical writing mode, a flex item with non-zero block paddings causes its flex container become bigger than expected (Firefox mixes up item's top/bottom vs. left/right padding when sizing flex container)

Categories

(Core :: Layout: Flexbox, defect, P3)

defect

Tracking

()

VERIFIED FIXED
mozilla64
Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 --- wontfix
firefox61 --- wontfix
firefox62 --- wontfix
firefox63 --- fixed
firefox64 --- verified

People

(Reporter: zjz, Assigned: dholbert)

References

(Blocks 1 open bug)

Details

(Keywords: regression, testcase)

Attachments

(3 files)

Attached file test.html
In vertical writing mode, a flex item with non-zero block paddings causes its flex container become bigger than expected

Please see the uploaded test.html
I just tested adding also non-zero inline paddings, and found that 

the width of the extra red part = padding-block-start + padding-block-end - padding-inline-start - padding-inline-end
Thanks for the report. This does seem to be a bug, and it started with this regression range:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=35dfa0882568c06f2d81b6749179815cd4f82886&tochange=dfd0afe71bb5c1430c3abca4ca31ac76d65cc1f1

--> regression from bug 1267462, new in Firefox 60. (That bug fixed up a bunch of vertical writing mode stuff, but apparently also caused this breakage.)
Assignee: nobody → dholbert
Blocks: 1267462
Status: NEW → ASSIGNED
Has Regression Range: --- → yes
Flags: needinfo?(dholbert)
Keywords: regression, testcase
(In reply to 張俊芝(Zhang Junzhi) from comment #1)
> I just tested adding also non-zero inline paddings, and found that 
> 
> the width of the extra red part = padding-block-start + padding-block-end -
> padding-inline-start - padding-inline-end

This was pretty much spot on. There was a left-over TopBottom() in the calculations that we needed to logicalize here.
(working on an autmated test for this - here's what I've got at the moment, trying to cover a bunch of edge cases)
Summary: In vertical writing mode, a flex item with non-zero block paddings causes its flex container become bigger than expected → In vertical writing mode, a flex item with non-zero block paddings causes its flex container become bigger than expected (Firefox mixes up item's top/bottom vs. left/right padding when sizing flex container)
Mats, not sure if you saw the phabricator review -- this is pending r?you over there at https://phabricator.services.mozilla.com/D4974
Flags: needinfo?(mats)
Comment on attachment 9006366 [details]
Bug 1486086: Switch to use logical axes, for stale physical-axis-based flex-item border/padding calculation.

Mats Palmgren (:mats) has approved the revision.
Attachment #9006366 - Flags: review+
Pushed by dholbert@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7b420ac55483
Switch to use logical axes, for stale physical-axis-based flex-item border/padding calculation. r=mats
Ryan, since it has a regression, could this be uplifted to beta?
Flags: needinfo?(ryanvm)
(In reply to 張俊芝(Zhang Junzhi) from comment #10)
> Ryan, since it has a regression, could this be uplifted to beta?

This is a fairly old regression (regressed in Firefox 60, and shipped broken in 60, 61, and 62) so I don't think that in and of itself is a strong argument for an uplift.

However: I do think this should be reasonable to uplift, since it's a small targeted patch, and is pretty obviously correct, and we're at the beginning of the Beta cycle.

I'll wait a day or two (to get a little Nightly testing / verification) and then I'll submit the beta uplift request.
Flags: needinfo?(ryanvm)
Flags: needinfo?(mats)
Flags: needinfo?(dholbert)
Thanks. Great, glad to hear that it will be uplifted.
https://hg.mozilla.org/mozilla-central/rev/7b420ac55483
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Just verified that yesterday's nightly is broken (red visible), vs. today's nightly is fixed (no red visible) on this bug's testcase: https://bugzilla.mozilla.org/attachment.cgi?id=9003865
Comment on attachment 9006366 [details]
Bug 1486086: Switch to use logical axes, for stale physical-axis-based flex-item border/padding calculation.

Requesting beta uplift.

Approval Request Comment
[Feature/Bug causing the regression]: bug 1267462
[User impact if declined]: broken layout, if the site uses the right combination of features (flexbox, vertical writing mode, different padding on different sides of a flex item)
[Is this code covered by automated tests?]: Yes.
[Has the fix been verified in Nightly?]: Yes.
[Needs manual test from QE? If yes, steps to reproduce]:  No.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: No.
[Why is the change risky/not risky?]: It only changes behavior for vertical writing-modes, which still aren't used much on the web. And it changes behavior in a very small, targeted, easy-to-validate-correctness way.
[String changes made/needed]: None.
Flags: needinfo?(dholbert)
Attachment #9006366 - Flags: approval-mozilla-beta?
Comment on attachment 9006366 [details]
Bug 1486086: Switch to use logical axes, for stale physical-axis-based flex-item border/padding calculation.

Targeted fix with tests verified on Nightly, uplift approved for 63 beta 6.
Attachment #9006366 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.