546530 - "ASSERTION: Attempting to allocate excessively large array" with execCommand("outdent"), combining acute accent

Status: RESOLVED FIXED

(Core :: Layout: Text and Fonts, defect)

Description

[(no longer active)]

+++ This bug was initially created as a clone of Bug #499844 +++

Testcase: https://bugzilla.mozilla.org/attachment.cgi?id=384527

###!!! ASSERTION: Attempting to allocate excessively large array: 'Error', file nsTArray.cpp, line 69

See bug 499844 comment 14, 16 and 17.

Comment 1

[(no longer active)]

Can someone with security access make this block on bug 377438 as well?  I couldn't do it because I don't have the access.

Comment 2

[(no longer active)]

(In reply to bug 499844 comment #17)
> If you can explain the results of your debugging in more detail, we can
> probably work out what to do. But it might be simpler just to reassign the
> second assertion to me.

Well, there were merely observations, no results per se.

After the FindClusterStart call, the properties object's iterator start becomes 0, whereas it's 1 for iter.  Therefore, the GetSkippedDistance call returns -1.  I think that it's kind of meaningless to call GetSkippedDistance with those parameters after the FindClusterStart call, but I might be wrong.  Or maybe the code is silently assuming that FindClusterStart can never modify the iterator's start in this way, but reading its implementation, it seems sort of obvious that it does.

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

OK. FindClusterStart walks backwards looking for the start of a cluster. The problem here is that it's walking backwards beyond the start of the text frame. We need to make it stop moving backwards when it reaches the start of the text frame.

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

Comment 5

[Simon Montagu :smontagu]

Comment on attachment 427246 [details] [diff] [review]
fix

>@@ -2460,17 +2460,17 @@ PropertyProvider::GetSpacingInternal(PRU

>         PRInt32 originalOffset = run.GetOriginalOffset() + i;
>         if (IsJustifiableCharacter(mFrag, originalOffset, isCJK)) {
>           iter.SetOriginalOffset(originalOffset);
>-          FindClusterStart(mTextRun, &iter);
>+          FindClusterStart(mTextRun, run.GetOriginalOffset(), &iter);
>           PRUint32 clusterFirstChar = iter.GetSkippedOffset();
>           FindClusterEnd(mTextRun, run.GetOriginalOffset() + run.GetRunLength(), &iter);

If you're already touching this code ... I found it hard to follow which was which of originalOffset and run.GetOriginalOffset(). Can you rename the local to iterOriginalOffset or something (or maybe use two locals, runOriginalOffset and iterOriginalOffset?)

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Sure.

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

http://hg.mozilla.org/mozilla-central/rev/006e1e7dd7c1

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 427246 [details] [diff] [review]
fix

Should be easy and safe to backport to branches.

Comment 9

[Daniel Veditz [:dveditz]]

Do we need this in 1.9.0 too like bug 499844?

Comment 10

[Daniel Veditz [:dveditz]]

Comment on attachment 427246 [details] [diff] [review]
fix

Approved for 1.9.1.9 and 1.9.2.2, a=dveditz for release-drivers

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 427246 [details] [diff] [review]
fix

Yes.

Comment 12

[Robert O'Callahan (:roc) (email my personal email if necessary)]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/36b685937fc6

Comment 13

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 427246 [details] [diff] [review]
fix

a=beltzner for 1.9.0.19

Comment 14

[(no longer active)]

The patch does not apply cleanly on either 1.9.1 or 1.9.0.

Comment 15

[Robert O'Callahan (:roc) (email my personal email if necessary)]

There's a hunk in there that was cleaning up warnings. It's not needed for 1.9.0/1.9.1.

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/a0091c234b2b
Also checked into 1.9.0 CVS.

Comment 16

[Al Billings [:abillings - ex-MoCo]]

Verified for 1.9.1 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.1.9pre) Gecko/20100312 Shiretoko/3.5.9pre. 

Verified for 1.9.2 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.2.2pre) Gecko/20100312 Namoroka/3.6.2pre. 

One 1.9.0, built today, when I run the testcase (https://bugzilla.mozilla.org/attachment.cgi?id=384527), I see:

###!!! ASSERTION: Attempting to allocate excessively large array: 'Error', file nsTArray.cpp, line 69

So, it looks like it isn't fixed on 1.9.0.