1162813 - ASSERTION: Unexpected paragraph separator: 'Not Reached', file layout/base/nsBidi.cpp, line 549

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Jesse Ruderman]

Attachment: testcase for Firefox

###!!! ASSERTION: Unexpected paragraph separator: 'Not Reached', file layout/base/nsBidi.cpp, line 557

Comment 1

[Jesse Ruderman]

Attachment: stack

Comment 2

[Jesse Ruderman]

Attachment: failed attempt to create a content testcase

Comment 3

[Simon Montagu :smontagu]

(In reply to Jesse Ruderman from comment #2)
> failed attempt to create a content testcase

It failed because we filter all characters with Bidi type Segment Separator or Block Separator in nsBidiPresUtils::ResolveParagraph. We probably need to do the same in the other callers of SetPara.

Comment 4

[Simon Montagu :smontagu]

(But sooner or later we probably want to add support for multiple paragraphs in nsBidi anyway. I'll open another bug about that)

Comment 5

[Chris Peterson [:cpeterson]]

I see this assertion failure when scrolling through about:config in a debug build:

[45253] ###!!! ASSERTION: Unexpected paragraph separator: 'Not Reached', file layout/base/nsBidi.cpp, line 549
[45253] ###!!! ASSERTION: Unexpected paragraph separator: 'Not Reached', file layout/base/nsBidi.cpp, line 1151
[45253] ###!!! ASSERTION: Unexpected paragraph separator: 'Not Reached', file layout/base/nsBidi.cpp, line 549
[45253] ###!!! ASSERTION: Unexpected paragraph separator: 'Not Reached', file layout/base/nsBidi.cpp, line 549

Comment 6

[Simon Montagu :smontagu]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a738b8e3b12b

Comment 7

[Simon Montagu :smontagu]

Attachment: crashtest

Comment 8

[Simon Montagu :smontagu]

Attachment: Fix

What I suggested in comment 3.

FTR, multi-paragraph support is bug 1163923.

Comment 9

[Jonathan Kew [:jfkthame]]

Comment on attachment 8695216 [details] [diff] [review]
Fix

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

::: layout/base/nsBidiPresUtils.cpp
@@ +2009,5 @@
>    NS_ASSERTION((aPosResolve == nullptr) != (aPosResolveCount > 0), "Incorrect aPosResolve / aPosResolveCount arguments");
>  
>    int32_t runCount;
>  
>    nsAutoString textBuffer(aText, aLength);

Huh - it seems that in current trunk code (before your patch), this |textBuffer| string was completely unused. I wonder if the compiler was smart enough to optimize it away?

(Looks like that dates back to bug 624798, which replaced the old mBuffer member variable with this local, but then didn't use it for anything.)

Comment 10

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3fcd9e467788
https://hg.mozilla.org/integration/mozilla-inbound/rev/d08afef8b42d

Comment 11

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/3fcd9e467788
https://hg.mozilla.org/mozilla-central/rev/d08afef8b42d