If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

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

VERIFIED FIXED in mozilla1.9alpha1

Status

()

Core
Layout: Text
--
critical
VERIFIED FIXED
11 years ago
6 years ago

People

(Reporter: Martijn Wargers (dead), Assigned: Uri Bernstein (Google))

Tracking

({crash, regression, testcase})

Trunk
mozilla1.9alpha1
crash, regression, testcase
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(crash signature)

Attachments

(3 attachments, 2 obsolete attachments)

(Reporter)

Description

11 years ago
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.
(Reporter)

Updated

11 years ago
Assignee: mozilla → nobody
Component: MailNews: BiDi Hebrew & Arabic → Layout: BiDi Hebrew & Arabic
QA Contact: giladehven → layout.bidi
(Reporter)

Comment 1

11 years ago
Created attachment 230293 [details]
testcase

This testcase crashes for me when following the steps to reproduce.
(Assignee)

Comment 2

11 years ago
This is an ugly one... taking.
Assignee: nobody → uriber
OS: Windows XP → All
Hardware: PC → All
(Assignee)

Comment 3

11 years ago
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
(Reporter)

Comment 4

11 years ago
Created attachment 230314 [details]
automated testcase (needs to be run locally)

(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.
(Assignee)

Comment 5

11 years ago
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.
(Assignee)

Comment 6

11 years ago
Created attachment 230319 [details] [diff] [review]
patch A (diff -w)

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)
(Assignee)

Comment 7

11 years ago
Created attachment 230321 [details] [diff] [review]
patch B

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)
(Assignee)

Comment 8

11 years ago
Created attachment 230337 [details] [diff] [review]
Patch A v2 (diff -w)

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)

Updated

11 years ago
Attachment #230337 - Flags: review?(smontagu) → review+
(Assignee)

Updated

11 years ago
Attachment #230337 - Flags: superreview?(roc)
Attachment #230337 - Flags: superreview?(roc) → superreview+
(Assignee)

Comment 9

11 years ago
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
Last Resolved: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9alpha
(Reporter)

Comment 10

11 years ago
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

Updated

9 years ago
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.