Closed Bug 1001233 Opened 6 years ago Closed 6 years ago

"ASSERTION: writing-mode mismatch" with li::-moz-list-bullet { direction: rtl; }

Categories

(Core :: Layout, defect)

x86_64
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla32
Tracking Status
firefox29 --- unaffected
firefox30 --- fixed
firefox31 --- fixed
firefox32 --- fixed
b2g-v1.4 --- fixed
b2g-v2.0 --- fixed

People

(Reporter: jruderman, Assigned: jfkthame)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, regression, testcase, Whiteboard: [qa-] )

Attachments

(4 files)

Attached file testcase
(Another assertion like in bug 986899)

###!!! ASSERTION: writing-mode mismatch: 'aWritingMode == mWritingMode', file WritingModes.h, line 732
Attached file stack
Attached patch PatchSplinter Review
This patch prevents the assertions. I'm not quite sure if the rendering is correct, but I don't think this is a case we really have to care about in the real world.
Assignee: nobody → smontagu
Attachment #8420652 - Flags: review?(jfkthame)
Comment on attachment 8420652 [details] [diff] [review]
Patch

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

::: layout/generic/nsBlockFrame.cpp
@@ +6879,5 @@
> +  LogicalRect logicalFAS(blockWM, floatAvailSpace, containerWidth);
> +  nscoord iStart = logicalFAS.IStart(blockWM) -
> +                   rs.ComputedLogicalBorderPadding().IStart(blockWM) -
> +                   reflowState.ComputedLogicalMargin().IEnd(bulletWM) -
> +                   aMetrics.ISize();

No, this can't be right - it's arithmetically combining logical coordinates in (potentially) different writing modes, which is meaningless.

The error becomes visible, for example, if you add some content to the testcase and include
  margin-right:1em;
to the li::-moz-list-bullet style: the margin fails to show up to the right of the bullet.
Attachment #8420652 - Flags: review?(jfkthame) → review-
Here's what I think we actually need to do in this case.
Attachment #8420916 - Flags: review?(smontagu)
Attachment #8420916 - Flags: review?(smontagu) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/1e15bbf34b27
Assignee: smontagu → jfkthame
Target Milestone: --- → mozilla32
The faulty code here was part of patch 1 in bug 789096, and as such it's now on aurora and beta branches as well. So we should consider uplifting the fix here once it's safely in Nightly.
https://hg.mozilla.org/mozilla-central/rev/1e15bbf34b27
Status: NEW → RESOLVED
Closed: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Comment on attachment 8420916 [details] [diff] [review]
convert bullet frame's margin to the block frame's writing mode when positioning bullet.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 789096 (patch 1)

User impact if declined: Possibility of misplaced or missing bullets in (unusual) bidi cases.

Testing completed (on m-c, etc.): Landed on m-c; includes testcase.

Risk to taking this patch (and alternatives if risky): Minimal risk: there's no security issue involved here, AFAICS, only the possibility of the bullet being placed wrongly. Patch is extremely localized and does not affect anything else apart from positioning of the bullet frame.

String or IDL/UUID changes made by this patch: none
Attachment #8420916 - Flags: approval-mozilla-beta?
Attachment #8420916 - Flags: approval-mozilla-aurora?
Attachment #8420916 - Flags: approval-mozilla-beta?
Attachment #8420916 - Flags: approval-mozilla-beta+
Attachment #8420916 - Flags: approval-mozilla-aurora?
Attachment #8420916 - Flags: approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.