Crash [@ BidiParagraphData::AppendFrame] removing RTL text

RESOLVED FIXED in Firefox 10

Status

()

Core
Layout: Text
--
critical
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: Jesse Ruderman, Assigned: smontagu)

Tracking

(Blocks: 1 bug, 4 keywords)

Trunk
mozilla11
x86_64
Mac OS X
crash, regression, rtl, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox9 unaffected, firefox10+ verified)

Details

(Whiteboard: [qa+][qa!:10], crash signature)

Attachments

(3 attachments)

(Reporter)

Description

6 years ago
Created attachment 578517 [details]
testcase (crashes Firefox when loaded)
(Reporter)

Comment 1

6 years ago
Created attachment 578518 [details]
stack trace
(Reporter)

Comment 2

6 years ago
crash-stats shows about 7 crashes a day with this signature. I don't know if those crashes are the same as this bug.
(Assignee)

Comment 3

6 years ago
I can't reproduce this in my local build on Linux, so either it is platform-specific (which seems unlikely), or a patch for some other bug that isn't yet checked in fixes it. I'll investigate further.
Assignee: nobody → smontagu
(Assignee)

Comment 4

6 years ago
Created attachment 579968 [details] [diff] [review]
Patch

So what is happening here is that after removing the span we have a frametree like this:

Inline(bdi)(0)@0xaafccaa0 next=0xaafccf80 next-continuation=0xaafccf80 {0,0,660,1140} [state=0000000000601000] [content=0xaafca650] [sc=0xaafcc878]<
  Inline(bdi)(0)@0xaafccae8 next-continuation=0xaafccf38 {0,0,660,1140} [state=0000000000601000] [content=0xaafca6a0] [sc=0xaafcc940]<>
>
Inline(bdi)(0)@0xaafccf80 prev-continuation=0xaafccaa0 {660,0,0,1140} [state=0000000000a00004] [content=0xaafca650] [sc=0xaafcc878]<
  Inline(bdi)(0)@0xaafccf38 prev-continuation=0xaafccae8 {0,0,0,1140} [state=0000000000a00000] [content=0xaafca6a0] [sc=0xaafcc940]<
    Text(0)" "@0xaafccbc0 [run=(nil)][0,1,T]  {0,0,0,1140} [state=0000000088420000] [content=0xaafca790] sc=0xaafcca48 pst=:-moz-non-element
  >
>

Since the first nested <bdi> is empty, TraverseFrames doesn't recurse into it, and then when it reaches the second nested <bdi>, it isn't a first continuation so it doesn't call Reset, and the data in the subparagraph BidiParagraphData never gets initialized. (and by the way, in spite of what I said in comment 3, this *is* platform specific: it depends on how the platform initializes the non-initialized data).
Attachment #579968 - Flags: review?
(Assignee)

Updated

6 years ago
Attachment #579968 - Flags: review? → review?(roc)
(Assignee)

Updated

6 years ago
Blocks: 613149
status-firefox10: --- → affected
status-firefox9: --- → unaffected
tracking-firefox10: --- → ?
Keywords: regression, rtl
Attachment #579968 - Flags: review?(roc) → review+
(Assignee)

Comment 5

6 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/1e0d0ad9767a
Flags: in-testsuite+

Comment 6

6 years ago
https://hg.mozilla.org/mozilla-central/rev/1e0d0ad9767a
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla11

Comment 7

6 years ago
[triage comment]
Regression introduce in Firefox 10. Tracking.

Please nominate this for beta if the patch is appropriate
tracking-firefox10: ? → +
(Assignee)

Updated

6 years ago
Attachment #579968 - Flags: approval-mozilla-beta?

Comment 8

6 years ago
Comment on attachment 579968 [details] [diff] [review]
Patch

[Triage Comment]
7 crashes per day on m-c/aurora warrants fixing this FF10 regression on beta at this point in the cycle. Approved.
Attachment #579968 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
(Assignee)

Comment 9

6 years ago
https://hg.mozilla.org/releases/mozilla-beta/rev/25c21764430d
status-firefox10: affected → fixed
Whiteboard: [qa+]

Comment 10

6 years ago
I've loaded the testcase from the attachament on:
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:10.0) Gecko/20100101 Firefox/10.0 beta 3 and I got no crash

Verified fixed on Beta.
status-firefox10: fixed → verified
Whiteboard: [qa+] → [qa+][qa!:10]
You need to log in before you can comment on or make changes to this bug.