986899 - "ASSERTION: writing-mode mismatch" with pre-line, rtl

Status: VERIFIED FIXED

(Core :: Layout: Block and Inline, defect)

Description

[Jesse Ruderman]

Attachment: testcase

###!!! ASSERTION: writing-mode mismatch: 'aWritingMode == mWritingMode', file layout/generic/WritingModes.h

Comment 1

[Jonathan Kew [:jfkthame]]

Attachment: use writing mode from mRootSpan to avoid mismatch assertions

The assertions here come from nsLineLayout::ApplyFrameJustification, where the PerSpanData's writing mode does not match that of the PerFrameData's bounds. Simply using the mode from mRootSpan instead of aPSD makes the assertion go away, but may not actually be correct - I haven't thought it all through at this point. This doesn't appear to break existing tests (https://tbpl.mozilla.org/?tree=Try&rev=9cca896ddd1d), but that may simply mean our test coverage is incomplete! Should we instead be explictly handling writing mode mismatches here, so as to convert coordinates if necessary?

Comment 2

[Simon Montagu :smontagu]

I tested the same fix in https://tbpl.mozilla.org/?tree=Try&rev=27fe3212b2e3 :). I believe this *is* the correct fix: all of the action in ApplyFrameJustification needs to be in the inline-direction of the block, even if child spans have a different direction. Orthogonal flows are not an exception, because an inline element with a different block-flow direction from its parent automatically becomes inline-block, so it won't go through this recursive code path anyway.

Comment 3

[Simon Montagu :smontagu]

Attachment: Patch with testcase

By the way, I also checked that there isn't any other case in nsLineLayout where we use psd->mWritingMode for the line writing mode and psd is not the root span.

Comment 4

[Jonathan Kew [:jfkthame]]

Comment on attachment 8395711 [details] [diff] [review]
Patch with testcase

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

Sounds good then (i.e. comment 2); thanks for checking.

BTW, when writing the check-in comment, beware of confusing mc-merge with multiple bug numbers as per bug 983691. :)

Comment 5

[Simon Montagu :smontagu]

https://hg.mozilla.org/integration/mozilla-inbound/rev/769d7d20adf4

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/769d7d20adf4

Comment 7

[Paul Silaghi, QA [:pauly]]

Reproduced in Nightly 2014-03-24-mozilla-central-debug.
Verified fixed FF 31.0a1 Nightly 2014-03-31-mozilla-central-debug, Win 7 x64.