Add reftests for "display:-webkit-box" & related properties

RESOLVED FIXED in Firefox 46

Status

()

Core
Layout
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: dholbert, Assigned: dholbert)

Tracking

Trunk
mozilla46
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox46 fixed)

Details

Attachments

(3 attachments, 1 obsolete attachment)

(Assignee)

Description

2 years ago
Filing this bug to add some reftests for "display: -webkit-box" and associated properties from bug 1208344 & bug 1208635.
(Assignee)

Comment 1

2 years ago
Created attachment 8701620 [details] [diff] [review]
part 1: add implicitly horizontal testcases, to exercise all -webkit-box-align & -webkit-box-pack values

This patch adds 2 testcases & their reference cases, testing all the various values of -webkit-box-pack & -webkit-box-align. The references are the same, but with the modern-flexbox equivalents (which we map to under the hood).

These testcases have "-webkit-box-orient" unset, so they're implicitly horizontal.

Subsequent patches here will add additional versions of these tests (using "hg cp") with -webkit-box-orient set to horizontal or vertical.

NOTE: The piece of the reftest about "-webkit-box-pack: justify" is currently "display:none" with an XXX comment, since it doesn't pass yet -- that'll be coming in bug 1231682.  (I chose to use "display:none" instead of "fails ==" in the interests of making this test comprehensive [testing all the values] & also making it actively test the values that are implemented [not marking it "fails"].)
(Assignee)

Updated

2 years ago
Attachment #8701620 - Flags: review?(cam)
(Assignee)

Updated

2 years ago
Assignee: nobody → dholbert
(Assignee)

Comment 2

2 years ago
Created attachment 8701622 [details] [diff] [review]
part 2: add explicitly-horizontal testcase variants (with "hg cp")
Attachment #8701622 - Flags: review?(cam)
(Assignee)

Comment 3

2 years ago
Created attachment 8701624 [details] [diff] [review]
part 3: add explicitly-vertical testcase & reference cases (with "hg cp")
Attachment #8701624 - Flags: review?(cam)
Comment on attachment 8701620 [details] [diff] [review]
part 1: add implicitly horizontal testcases, to exercise all -webkit-box-align & -webkit-box-pack values

Review of attachment 8701620 [details] [diff] [review]:
-----------------------------------------------------------------

The difference in rendering for the difference box-align values in webkit-box-align-horiz-1a.html is really slight -- 0.5 or 1.5 pixels.  Can we make the two font sizes vastly different so that it's more obvious what's going on?
(Assignee)

Comment 5

2 years ago
Good point -- the differences are clearer with "font-size: 50%" (instead of the "80%" that the test uses for the second flex item right now).

I'll repost part 1, with that changed in both tests.
(Assignee)

Comment 6

2 years ago
Created attachment 8701652 [details] [diff] [review]
part 1 v2 (with 50% instead of 80% font-size in second flex item)

Here's part 1 with 50% instead of 80% font-size in the second flex item.

Parts 2 and 3 still apply cleanly (and will pick up this change by the magic of 'hg cp')
Attachment #8701620 - Attachment is obsolete: true
Attachment #8701620 - Flags: review?(cam)
Attachment #8701652 - Flags: review?(cam)
(Assignee)

Comment 7

2 years ago
(Note also that part 3's vertical tests won't pass in current Nightly -- they need the fix for bug 1208344 (which is marked as a dependency).)
Attachment #8701652 - Flags: review?(cam) → review+
Comment on attachment 8701622 [details] [diff] [review]
part 2: add explicitly-horizontal testcase variants (with "hg cp")

Review of attachment 8701622 [details] [diff] [review]:
-----------------------------------------------------------------

(Thanks for the use of hg cp; makes it easier to review!)
Attachment #8701622 - Flags: review?(cam) → review+
Attachment #8701624 - Flags: review?(cam) → review+
Don't forget to add webkit-box/reftest.list to layout/reftests/reftest.list.
(Assignee)

Comment 10

2 years ago
Ah, good catch -- thanks! I'd been testing the reftests locally via targeted runs (passing the exact directory name to ./mach reftest).

Fixed that locally.

Comment 12

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/3d715941e8d7
https://hg.mozilla.org/mozilla-central/rev/f69d91bcc0d6
https://hg.mozilla.org/mozilla-central/rev/a1d32f3194a4
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox46: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
You need to log in before you can comment on or make changes to this bug.