Closed Bug 1581718 Opened 5 years ago Closed 5 years ago

floating tables with orthogonal writing mode have negative parent-inline-end margin

Categories

(Core :: Layout, defect)

Product:
Core
Component:
Layout
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla71
Tracking Flags:
Tracking Status
firefox71 --- fixed

People

(Reporter: dbaron, Assigned: dbaron)

References

Details

Attachments

(2 files, 1 obsolete file)

debuggable testcase
1.01 KB, text/html
Details
Bug 1581718 - Avoid strange results for orthogonal-flow cases in CalculateBlockSideMargins when border/padding/margin are present.
47 bytes, text/x-phabricator-request
Details | Review

The web-platform-tests:
css/css-writing-modes/table-progression-vlr-004.html
css/css-writing-modes/table-progression-vrl-004.html
have too small an amount of space between the tables.

This space is too small because while the table box is 24px wide, the table wrapper box is 18px wide. This difference is coming from a negative margin that I've so far traced back to this code.

I suspect the underlying problem is that the table wrapper box is a float, floats are sized based on their intrinsic width, and we're getting "incorrect" intrinsic width computation across the orthogonal writing mode (where it's now an "intrinsic height").

So far I haven't looked into this further.

I think the problem may well be that this code doesn't consider borderPadding, so we end up subtracting the sum of borderPadding and margin from the margin-inline-end.

(I'd also note that in this case both computedISizeCBWM and availISizeCBWM are NS_UNCONSTRAINEDSIZE.)

Here's a new try run with more failure annotations removed.

Assignee: nobody → dbaron
Status: NEW → ASSIGNED

And to be clear, the analysis in comment 2 is what the fix is based on -- I think the problem was that it was making two things equal when it intended to make them differ by the padding, border, and margin that would be subtracted from one of them -- which would in turn just make the function return early. So why not just return earlier?

Pushed by dbaron@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/31c54bb6faff Avoid strange results for orthogonal-flow cases in CalculateBlockSideMargins when border/padding/margin are present. r=jfkthame

This bug's main patch seems to have reduced the assertion-count for this test
to 1, at least on some platforms, so let's allow that in the annotaiton.

Here's the TEST-UNEXPECTED-PASS orange-log that spawned the followup patch in comment 9:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=267060505&repo=autoland&lineNumber=38301

Pushed by dholbert@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/03ed5549fb82 followup: Adjust expected assertion count for crashtest '1460787-1.html'. (no review, test-annotation-only)

Yeah, looks like the assertion count can be as low as 0 now. It was originally asserts(2-4), but as of this patch, it looks like 0 and 1 have both been observed as the assertion-count.

Maybe 2 (or more) of the original 2-4 assertions are fixed by this patch?

Attachment #9093394 - Attachment is obsolete: true

I think the assertion count unexpected pass in reftest/tests/layout/generic/crashtests/334148-1.xhtml was unrelated to this patch and started quite a bit later. It's a different test from the one dholbert fixed; it looks to me that for this patch, dholbert's fix was sufficient.

Pushed by dbaron@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/34048312a108 Avoid strange results for orthogonal-flow cases in CalculateBlockSideMargins when border/padding/margin are present. r=jfkthame

https://hg.mozilla.org/mozilla-central/rev/34048312a108

Status: ASSIGNED → RESOLVED
Closed: 5 years ago
status-firefox71: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla71
Flags: needinfo?(dbaron)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: