Last Comment Bug 707098 - Crash [@ BidiParagraphData::AppendFrame] removing RTL text
: Crash [@ BidiParagraphData::AppendFrame] removing RTL text
Status: RESOLVED FIXED
[qa+][qa!:10]
: crash, regression, rtl, testcase
Product: Core
Classification: Components
Component: Layout: Text (show other bugs)
: Trunk
: x86_64 Mac OS X
: -- critical (vote)
: mozilla11
Assigned To: Simon Montagu :smontagu
:
:
Mentors:
Depends on:
Blocks: stirdom 613149
  Show dependency treegraph
 
Reported: 2011-12-02 01:23 PST by Jesse Ruderman
Modified: 2012-01-09 02:54 PST (History)
6 users (show)
smontagu: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
unaffected
+
verified


Attachments
testcase (crashes Firefox when loaded) (185 bytes, text/html)
2011-12-02 01:23 PST, Jesse Ruderman
no flags Details
stack trace (9.35 KB, text/plain)
2011-12-02 01:23 PST, Jesse Ruderman
no flags Details
Patch (3.69 KB, patch)
2011-12-07 22:42 PST, Simon Montagu :smontagu
roc: review+
akeybl: approval‑mozilla‑beta+
Details | Diff | Splinter Review

Description Jesse Ruderman 2011-12-02 01:23:09 PST
Created attachment 578517 [details]
testcase (crashes Firefox when loaded)
Comment 1 Jesse Ruderman 2011-12-02 01:23:37 PST
Created attachment 578518 [details]
stack trace
Comment 2 Jesse Ruderman 2011-12-02 01:30:42 PST
crash-stats shows about 7 crashes a day with this signature. I don't know if those crashes are the same as this bug.
Comment 3 Simon Montagu :smontagu 2011-12-02 06:12:12 PST
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.
Comment 4 Simon Montagu :smontagu 2011-12-07 22:42:40 PST
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).
Comment 6 Ed Morley [:emorley] 2011-12-08 08:27:05 PST
https://hg.mozilla.org/mozilla-central/rev/1e0d0ad9767a
Comment 7 christian 2011-12-22 14:56:23 PST
[triage comment]
Regression introduce in Firefox 10. Tracking.

Please nominate this for beta if the patch is appropriate
Comment 8 Alex Keybl [:akeybl] 2011-12-26 10:53:56 PST
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.
Comment 9 Simon Montagu :smontagu 2011-12-26 21:42:21 PST
https://hg.mozilla.org/releases/mozilla-beta/rev/25c21764430d
Comment 10 Vlad [QA] 2012-01-09 02:54:19 PST
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.

Note You need to log in before you can comment on or make changes to this bug.