Closed Bug 345616 Opened 18 years ago Closed 18 years ago

Pressing right arrow key crashes [@ IsBidiLeaf] Mozilla in this case

Categories

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

defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla1.9alpha1

People

(Reporter: martijn.martijn, Assigned: uriber)

References

Details

(Keywords: crash, regression, testcase)

Crash Data

Attachments

(3 files, 2 obsolete files)

See upcoming testcase, when following the steps in the testcase, I crash in current trunk build.
Talkback ID: TB21252165H
IsBidiLeaf   nsFrame::PeekOffset   nsFrameSelection::MoveCaret   nsFrameSelection::CharacterMove   nsSelectMoveScrollCommand::DoSelectCommand   nsSelectionCommandsBase::DoCommand   nsControllerCommandTable::DoCommand   nsBaseCommandController::DoCommand   nsXBLPrototypeHandler::ExecuteHandler  

This regressed between 2006-07-17 and 2006-07-18, I suspect a regression from bug 344514.
Assignee: mozilla → nobody
Component: MailNews: BiDi Hebrew & Arabic → Layout: BiDi Hebrew & Arabic
QA Contact: giladehven → layout.bidi
Attached file testcase
This testcase crashes for me when following the steps to reproduce.
This is an ugly one... taking.
Assignee: nobody → uriber
OS: Windows XP → All
Hardware: PC → All
For me, at least, this actually regressed between 2006-07-11 and 2006-07-12, due to bug 309432.

Martijn - could you please double-check?
Blocks: 309432
No longer blocks: 344514
Status: NEW → ASSIGNED
(In reply to comment #3)
> Martijn - could you please double-check?

I just checked again, I still get a 2006-07-17/2006-07-18 regression window. I don't crash with a 2006-07-12 build, for instance.
The patch for bug 309432 went in in 2006-07-12 04:22, so it's possible that it's not in your 2006-07-12 build (the Mac build was done later that day).

Anyway, I'll try to have a fix for the crash I'm seeing (which is certainly due to bug 309432), and then we'll have to make sure that it solves the problem on your system as well.
Attached patch patch A (diff -w) (obsolete) — Splinter Review
The patch for bug 309432 caused the code to rely on the content of the firstFrame and lastFrame output parameters of CheckLineOrder(), even if isReordered is FALSE, and made sure that nsLineIterator::CheckLineOrder() assigns values to these parameters in that case.
In our case, however, the method actually being called is not nsLineIterator::CheckLineOrder(), but nsTableRowGroupFrame::CheckLineOrder(), which simply sets isReordered is FALSE and leaves firstFrame and lastFrame untouched.

This patch deals with CheckLineOrder() returning null pointer(s) by assuming that in this case, we are at the edge of the line.
Attachment #230319 - Flags: review?(smontagu)
Attached patch patch B (obsolete) — Splinter Review
This patch makes nsTableRowGroupFrame::CheckLineOrder return its first and last children as aFirstVisual and aLastVisual.

Even if we take this patch, we might still want to take patch A as well, as a safeguard against empty lines or rowgroups (I'm not sure whether these can really exist).
Attachment #230321 - Flags: review?(smontagu)
Same as patch A, but set the output parameters to nsnull inside CheckLineOrder, instead of initializing them before the call and leaving them untouched by the method.
Attachment #230319 - Attachment is obsolete: true
Attachment #230321 - Attachment is obsolete: true
Attachment #230337 - Flags: review?(smontagu)
Attachment #230319 - Flags: review?(smontagu)
Attachment #230321 - Flags: review?(smontagu)
Attachment #230337 - Flags: review?(smontagu) → review+
Attachment #230337 - Flags: superreview?(roc)
Attachment #230337 - Flags: superreview?(roc) → superreview+
Checked in:
Checking in layout/generic/nsFrame.cpp;
/cvsroot/mozilla/layout/generic/nsFrame.cpp,v  <--  nsFrame.cpp
new revision: 3.663; previous revision: 3.662
done
Checking in layout/generic/nsLineBox.cpp;
/cvsroot/mozilla/layout/generic/nsLineBox.cpp,v  <--  nsLineBox.cpp
new revision: 1.112; previous revision: 1.111
done
Checking in layout/tables/nsTableRowGroupFrame.cpp;
/cvsroot/mozilla/layout/tables/nsTableRowGroupFrame.cpp,v  <--  nsTableRowGroupFrame.cpp
new revision: 3.365; previous revision: 3.364
done
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9alpha
Verified fixed, using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060804 Minefield/3.0a1
Status: RESOLVED → VERIFIED
Component: Layout: BiDi Hebrew & Arabic → Layout: Text
QA Contact: layout.bidi → layout.fonts-and-text
Crash Signature: [@ IsBidiLeaf]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: