330881 - Crash when double-clicking in empty area inside isindex with position:absolute;direction:rtl;

Status: VERIFIED FIXED

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

Description

[Martijn Wargers (dead)]

See upcoming testcase which crashes for me when double-clicking in the empty area at the left inside the isindex element
Basically the testcase consists of this:
<isindex style="position:absolute;direction:rtl;">

This regressed between 2006-03-11 and 2006-03-12:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2006-03-11+05&maxdate=2006-03-12+07&cvsroot=%2Fcvsroot
Regression from bug 303884, I guess.

Talkback ID: TB16509676Z
nsLineIterator::CheckLineOrder   nsFrame::GetFrameFromDirection   nsFrame::PeekOffset   nsFrame::PeekBackwardAndForward   0x029a1360

Comment 1

[Martijn Wargers (dead)]

Oops, meant to cc you, see bug.

Comment 2

[Martijn Wargers (dead)]

Attachment: testcase

Comment 3

[Uri Bernstein]

Attachment: patch

Here's the story:
I found that nsLineIterator::CheckLineOrder was receiving aLine=-1. The previous implementation (before bug 303884) somehow managed to deal with this (looking at line 0 instead) but this was obviously not the right thing to do.
So I added an assertion for aLine being in the correct range (and also made sure I don't crash on empty lines, while I was there), and moved on to find out why aLine was -1.

I found that this was because the clicked frame was an out-of-flow frame, and therefore it could not be found on any line of its containing block. I found elsewhere (in nsFrame::PeekOffset) code that deals with this situation exactly, and I also found several other places where basically the same thing ws done (searching for the containing block of a frame, and then finding which line of this block the frame is on). So I figures I'd write a small helper function which will do this, and also take care of the out-of-flow case.
But then I found GetLineNumber(), which does that exactly, but apparently was unused(!). I moved the out-of-flow handling code into it, and also made it return the containing block (which is needed for various reasons), and then changed the four places which had duplicated code to use it instead.

But then I found out that it still didn't work, because the out-of-flow code I was using was no good: It was only checking if the topmost frame (beneith the block) was an out-of-flow, and in my case a lower frame was an o-o-f. So I changed it to check for o-o-fs inside the loop.

Comment 4

[Uri Bernstein]

Checked in:

Checking in layout/generic/nsFrame.h;
/cvsroot/mozilla/layout/generic/nsFrame.h,v  <--  nsFrame.h
new revision: 3.243; previous revision: 3.242
done
Checking in layout/generic/nsFrame.cpp;
/cvsroot/mozilla/layout/generic/nsFrame.cpp,v  <--  nsFrame.cpp
new revision: 3.631; previous revision: 3.630
done
Checking in layout/generic/nsLineBox.cpp;
/cvsroot/mozilla/layout/generic/nsLineBox.cpp,v  <--  nsLineBox.cpp
new revision: 1.95; previous revision: 1.94
done

Comment 5

[Martijn Wargers (dead)]

Verified, using 2006-03-20 build.

Comment 6

[(no longer active)]

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