crash [@ nsEventStateManager::GetDocSelectionLocation]

VERIFIED FIXED in mozilla1.8.1beta2

Status

()

Core
Event Handling
--
critical
VERIFIED FIXED
12 years ago
12 years ago

People

(Reporter: mats, Assigned: mats)

Tracking

(4 keywords)

Trunk
mozilla1.8.1beta2
x86
All
crash, testcase, verified1.8.0.7, verified1.8.1
Points:
---
Bug Flags:
blocking1.8.1 +
blocking1.8.0.7 +

Firefox Tracking Flags

(Not tracked)

Details

(crash signature)

Attachments

(3 attachments)

(Assignee)

Description

12 years ago
STEPS TO REPRODUCE
1. load the attached testcase
2. click on the button
3. TAB twice  -> crash

PLATFORMS AND BUILDS TESTED
bug occurs with SeaMonkey trunk debug build on Linux
(Assignee)

Comment 1

12 years ago
Created attachment 229783 [details]
Testcase
(Assignee)

Comment 2

12 years ago
Created attachment 229784 [details]
stack
(Assignee)

Comment 3

12 years ago
Created attachment 229785 [details] [diff] [review]
Patch rev. 1

This fixes the crash but I'm not sure if it's actually the |domRange|
or root content, or something else that is incorrect... we have:

(gdb) fr 0
#0  0x4137d69e in nsEventStateManager::GetDocSelectionLocation(nsIContent**, nsIContent**, nsIFrame**, unsigned*) (this=0x88559c0,
    aStartContent=0xbfffd3e0, aEndContent=0xbfffd3d0, aStartFrame=0xbfffd2cc, aStartOffset=0xbfffd2d0) at nsEventStateManager.cpp:4783
4783          if (startContent->IsNodeOfType(nsINode::eELEMENT)) {
(gdb) p startNode
$6 = {mRawPtr = 0x83ce434}
(gdb) x/wa startNode.mRawPtr
0x83ce434:      0x4173c6c8 <_ZTV14nsHTMLDocument+1224>
(gdb) fr 1
#1  0x41379799 in nsEventStateManager::ShiftFocusInternal(int, nsIContent*) (this=0x88559c0, aForward=1, aStart=0x0)
    at nsEventStateManager.cpp:3406
3406          GetDocSelectionLocation(getter_AddRefs(selectionContent), getter_AddRefs(endSelectionContent), &selectionFrame, &selectionOffset);
(gdb) list
3401      if (!aStart && itemType != nsIDocShellTreeItem::typeChrome) {
3402        // We're going to tab from the selection position
3403        if (!mCurrentFocus || (mLastFocusedWith != eEventFocusedByMouse && mCurrentFocus->Tag() != nsHTMLAtoms::area)) {
3404          nsCOMPtr<nsIContent> selectionContent, endSelectionContent;  // We won't be using this, need arg for method call
3405          PRUint32 selectionOffset; // We won't be using this either, need arg for method call
3406          GetDocSelectionLocation(getter_AddRefs(selectionContent), getter_AddRefs(endSelectionContent), &selectionFrame, &selectionOffset);
3407          if (selectionContent == rootContent)  // If selection on rootContent, same as null -- we have no selection yet
3408            selectionFrame = nsnull;
3409          // Only use tabindex if selection is synchronized with focus
3410          // That way, if the user clicks in content, or does a find text that lands between focusable elements,
(gdb) p rootContent
$7 = {mRawPtr = 0x884d590}
(gdb) x/wa rootContent.mRawPtr
0x884d590:      0x41731a28 <_ZTV18nsHTMLInputElement+8>
Attachment #229785 - Flags: review?(bzbarsky)
Happens also on windows.
Comment on attachment 229785 [details] [diff] [review]
Patch rev. 1

This is the right fix, esp. for branch.  The range looks correct to me -- removing the root node will collapse all ranges to the Document node, which doesn't QI to nsIContent, of course.  The root content also looks correct.

That said, this code should probably be rewritten to use nsINode instead of nsIContent on trunk (at which point we could remove these null-checks).
Attachment #229785 - Flags: superreview+
Attachment #229785 - Flags: review?(bzbarsky)
Attachment #229785 - Flags: review+
Oh, and we probably need this on branches too...
Flags: blocking1.8.1?
Flags: blocking1.8.0.6?
(Assignee)

Comment 7

12 years ago
Checked in to trunk at 2006-07-19 22:25 PDT.
Filed bug 345278 on using nsINode instead of nsIContent.

-> FIXED
Status: NEW → RESOLVED
Last Resolved: 12 years ago
OS: Linux → All
Resolution: --- → FIXED
Flags: blocking1.8.1? → blocking1.8.1+
(Assignee)

Comment 8

12 years ago
Comment on attachment 229785 [details] [diff] [review]
Patch rev. 1

(In reply to comment #6)
> Oh, and we probably need this on branches too...

It doesn't crash in Fx2.0b1 or a local SeaMonkey branch debug build on Linux.
The reason being that |domRange| is null:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/content/events/src/nsEventStateManager.cpp&rev=MOZILLA_1_8_BRANCH&cvsroot=/cvsroot&mark=4819#4782

Maybe we should take it just in case that changes...
It's a simple null-check fix, zero risk.
Attachment #229785 - Flags: approval1.8.1?
Target Milestone: --- → mozilla1.8.1beta2
Comment on attachment 229785 [details] [diff] [review]
Patch rev. 1

a=drivers. Please land this on the MOZILLA_1_8_BRANCH.
Attachment #229785 - Flags: approval1.8.1? → approval1.8.1+
(Assignee)

Comment 10

12 years ago
Checked in to MOZILLA_1_8_BRANCH at 2006-07-28 11:45 PDT.
Keywords: fixed1.8.1
Flags: blocking1.8.0.7? → blocking1.8.0.7+
Comment on attachment 229785 [details] [diff] [review]
Patch rev. 1

approved for 1.8.0 branch, a=dveditz for drivers
Attachment #229785 - Flags: approval1.8.0.7+

Comment 12

12 years ago
I guess this wasn't happening before the patch went in, but nevertheless, it still isn't happening. Verified using Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1b1) Gecko/20060727 BonEcho/2.0b1 and the testcase from comment 1.
Status: RESOLVED → VERIFIED
Keywords: fixed1.8.1 → verified1.8.1
(Assignee)

Comment 13

12 years ago
Checked in to MOZILLA_1_8_0_BRANCH at 2006-08-21 10:49 PDT.
Keywords: fixed1.8.0.7
Same story for the 1.5.0.7 branch.
I don't get the crash with Firefox1.5.0.6, and I still don't get the crash, using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.7) Gecko/20060829 Firefox/1.5.0.7
Keywords: fixed1.8.0.7 → verified1.8.0.7
Crash Signature: [@ nsEventStateManager::GetDocSelectionLocation]
You need to log in before you can comment on or make changes to this bug.