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

RESOLVED FIXED in Firefox 45

Status

()

Core
Layout
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: Jesse Ruderman, Assigned: smontagu)

Tracking

(Blocks: 1 bug, 4 keywords)

Trunk
mozilla45
assertion, regression, reproducible, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox40 affected, firefox45 fixed)

Details

Attachments

(5 attachments)

(Reporter)

Description

3 years ago
Created attachment 8603093 [details]
testcase for Firefox

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

Comment 1

3 years ago
Created attachment 8603094 [details]
stack
(Reporter)

Comment 2

3 years ago
Created attachment 8603095 [details]
failed attempt to create a content testcase
(Assignee)

Comment 3

3 years ago
(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.
(Assignee)

Comment 4

3 years ago
(But sooner or later we probably want to add support for multiple paragraphs in nsBidi anyway. I'll open another bug about that)
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
status-firefox45: --- → affected
Keywords: reproducible
Summary: "ASSERTION: Unexpected paragraph separator" → ASSERTION: Unexpected paragraph separator: 'Not Reached', file layout/base/nsBidi.cpp, line 549
(Assignee)

Comment 6

2 years ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a738b8e3b12b
(Assignee)

Comment 7

2 years ago
Created attachment 8695214 [details] [diff] [review]
crashtest
Assignee: nobody → smontagu
Attachment #8695214 - Flags: review?(jfkthame)
(Assignee)

Comment 8

2 years ago
Created attachment 8695216 [details] [diff] [review]
Fix

What I suggested in comment 3.

FTR, multi-paragraph support is bug 1163923.
Attachment #8695216 - Flags: review?(jfkthame)
Attachment #8695214 - Flags: review?(jfkthame) → review+
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.)
Attachment #8695216 - Flags: review?(jfkthame) → review+

Comment 10

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

Comment 11

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/3fcd9e467788
https://hg.mozilla.org/mozilla-central/rev/d08afef8b42d
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox45: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
You need to log in before you can comment on or make changes to this bug.