crash [@ nsEventStateManager::GetDocSelectionLocation]

VERIFIED FIXED in mozilla1.8.1beta2



13 years ago
13 years ago


(Reporter: mats, Assigned: mats)


(4 keywords)

crash, testcase, verified1.8.0.7, verified1.8.1
Bug Flags:
blocking1.8.1 +
blocking1.8.0.7 +

Firefox Tracking Flags

(Not tracked)


(crash signature)


(3 attachments)



13 years ago
1. load the attached testcase
2. click on the button
3. TAB twice  -> crash

bug occurs with SeaMonkey trunk debug build on Linux

Comment 1

13 years ago
Created attachment 229783 [details]

Comment 2

13 years ago
Created attachment 229784 [details]

Comment 3

13 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?

Comment 7

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

Last Resolved: 13 years ago
OS: Linux → All
Resolution: --- → FIXED

Comment 8

13 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:

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+

Comment 10

13 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

13 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.
Keywords: fixed1.8.1 → verified1.8.1

Comment 13

13 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 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: Gecko/20060829 Firefox/
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.