Closed Bug 1178059 Opened 5 years ago Closed 5 years ago

investigate/fix failure in layout/reftests/writing-mode/tables/fixed-table-layout-010-vlr.html and ...010-vrl.html

Categories

(Core :: Layout: Tables, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla41
Tracking Status
firefox41 --- fixed

People

(Reporter: jfkthame, Assigned: smontagu)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

These testcases were among the set landed with bug 1173958, but tryserver said that test 010 would fail on most platforms (see bug 1173958 comment 21). It's not entirely clear if this indicates a real bug that should be fixed in code, or an overly-fragile test that should be rewritten more robustly.

For the time being, the two 010 testcases are marked as random. We should remove this annotation after investigating/fixing as needed.
Attached patch Patch (obsolete) — Splinter Review
Vertical alignment is hard...

writing-mode isn't involved here: the tests fail in the same way if I swap back the specified width and height and remove the vertical writing mode. The fix seems a bit like black magic to me, but I saw in more than one online source that the way to make a non-table element centre text vertically is to specify line-height (I would have expected |display: table-cell; vertical-align: middle| to have the same effect, but at least in this testcase it doesn't).

We might be able to make the test pass on Android too with a little juggling of the width and the font-size, but it doesn't seem worth investing the time right now.
Assignee: nobody → smontagu
Attachment #8627034 - Flags: review?(jfkthame)
Attached patch Patch v.2Splinter Review
rebased and fixed typo in "Android"
Attachment #8627034 - Attachment is obsolete: true
Attachment #8627034 - Flags: review?(jfkthame)
Attachment #8627036 - Flags: review?(jfkthame)
Comment on attachment 8627036 [details] [diff] [review]
Patch v.2

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

I don't really understand this, but whatever... it looks harmless in terms of what we're actually trying to test here, anyhow.
Attachment #8627036 - Flags: review?(jfkthame) → review+
https://hg.mozilla.org/mozilla-central/rev/a37ff62f34c7
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
You need to log in before you can comment on or make changes to this bug.