295142 - Crash when using option(ctrl)-left arrow at the end of RTL textarea [@nsFrame::PeekOffset]

Status: RESOLVED FIXED

(Core :: DOM: UI Events & Focus Handling, defect)

Description

[Uri Bernstein]

Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b2) Gecko/20050522
Firefox/1.0+

Steps to reproduce:

1. Go to http://oren.gomen.org/mozilla/bug_207186.html
2. Place the caret in the first textbox, on the last line containing text, to
the left of the leftmost period (i.e. at the end of the RTL row).
3. Press option-right arrow (ctrl-right arrow on PC)
4. Crash

I'll attach a mac crash dump soon. TB Incident ID is TB6042636Y.
I'm seeing this on OS X, but it's likely All/All (con someone confirm?)

Comment 1

[Uri Bernstein]

Attachment: crash log

Infinite recursion in nsFrame::PeekOffset / nsTextFrame::PeekOffset.

Notice the similarity to bug 256835.

Comment 2

[Uri Bernstein]

Adding regression keyword since this does not happen in Gecko 1.7 (FF 1.0).

Comment 3

[Uri Bernstein]

Attachment: minimal testcase

Place the caret in the (empty) textarea and hit option-left arrow => Crash.

Comment 4

[Matthias Versen [:Matti]]

*** Bug 292301 has been marked as a duplicate of this bug. ***

Comment 5

[Uri Bernstein]

Comment 0 set 3 should read "Press option-LEFT arrow (ctrl-LEFT arrow on PC)". I
really should be more careful.

Matti, did you reproduce this on a PC? If so, please provide a User-Agent string
and set the "Hardware" field to "All" as well. Thanks.

Comment 6

[Matthias Versen [:Matti]]

no, i didn't reproduce this

Comment 7

[Ginn Chen]

I reproduced it on both Linux and Mac OS X.
But I can't reproduce it on Windows.

Comment 8

[Ginn Chen]

It's something like Bug 274857.
Actually, if we move the #ifdef block out of 'if (aPos->mDirection == eDirNext)', it won't crash.
       if (aPos->mDirection == eDirNext)
       {
         aPos->mPreferLeft = (PRBool)!(aPos->mPreferLeft);//drift to other side
-#ifdef IBMBIDI
-        if (lineIsRTL)
-          aPos->mAmount = eSelectNoAmount;
-#endif
       }
+#ifdef IBMBIDI
+     if (lineIsRTL)
+        aPos->mAmount = eSelectNoAmount;
+#endif
But the Option+Left behaviour is still wrong, caret goes to the end of the next line.
I suggest we fix some issues in bug 207186 first, otherwise we're causing more regressions.

Comment 9

[Ginn Chen]

(In reply to comment #8)
> But the Option+Left behaviour is still wrong, caret goes to the end of the
next line.

Oops, it should be correct to go to next line if it is RTL.
I'm not quite sure about it.

Comment 10

[Matthias Versen [:Matti]]

The dupe is from win32

Comment 11

[Uri Bernstein]

> (In reply to comment #8)
> > But the Option+Left behaviour is still wrong, caret goes to the end of the
> next line.
> 
> Oops, it should be correct to go to next line if it is RTL.
> I'm not quite sure about it.

It goes to the RIGHT of the next line (which is the beginning of the line, if
you're writing in an RTL script - the more common case for an RTL textbox).
This is not exactly the correct behaviour - the caret should go to the end
(LEFT) of the first (RIGHTMOST) word in the next line - i.e. the mirror image of
what happens when you do an option-right at the end of a line in an LTR textbox.

It's still much better than crashing, though. So unless you're sure that some
serious work will be done on bug 207186 before the next release, IMHO the
solution/workaround in comment #8 should be checked in.

Comment 12

[Ginn Chen]

Attachment: patch to avoid crash

this patch is much like bug 274857.
I don't think we can fix all the BiDi bugs before next release.
So just avoid crash first.

The patch is safe.

Comment 13

[chris hofmann]

Comment on attachment 184562 [details] [diff] [review]
patch to avoid crash

a=chofmann

Comment 14

[Ginn Chen]

Checking in nsFrame.cpp;
/cvsroot/mozilla/layout/generic/nsFrame.cpp,v  <--  nsFrame.cpp
new revision: 3.564; previous revision: 3.563
done

Comment 15

[(no longer active)]

Mass-assigning the new rtl keyword to RTL-related (see bug 349193).