Closed Bug 1136735 Opened 9 years ago Closed 9 years ago

crash in nsINode::GetFirstElementChild() const with selectioncaret.enabled turned on on desktop

Categories

(Core :: DOM: Selection, defect)

Product:
Core
Component:
DOM: Selection
Platform:
All
macOS
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla39
Tracking Flags:
Tracking Status
firefox39 --- fixed

People

(Reporter: ehsan.akhgari, Assigned: TYLin)

Details

(Keywords: crash)

Crash Data

Attachments

(1 file)

Ignore events if there's no SelectionCarets elements. (v1)
1.17 KB, patch
ehsan.akhgari
: review+
mtseng
: feedback+
Details | Diff | Splinter Review 
This bug was filed from the Socorro interface and is 
report bp-feba3b8e-65c6-4877-8cdb-150482150225.
=============================================================

Steps to reproduce:
1. Enable selectioncaret.enabled on desktop (I tested on Mac), and restart.
2. In an open window, click on the location bar.

This is caused by PresShell::GetSelectionCaretsStartElement() returning null, and the caller (SelectionCarets::GetStartFrameRectInner) not testing its return value.

Ting-Yu, do you have time to fix this?  Should be an easy fix.  Thanks!
Flags: needinfo?(tlin)
Ehsan, I can reproduce this locally. Investigating.
Assignee: nobody → tlin
Status: NEW → ASSIGNED
Flags: needinfo?(tlin)
UL has no CanvasFrame. Hence no SelectionCarets elements.
SelectionCarets should ignore the events if it cannot get its elements.
Attachment #8569593 - Flags: feedback+
Attachment #8569593 - Flags: feedback+ → feedback?(mtseng)
Comment on attachment 8569593 [details] [diff] [review]
Ignore events if there's no SelectionCarets elements. (v1)

Looks good. Do we need same check in TouchCaret?
Attachment #8569593 - Flags: feedback?(mtseng) → feedback+
(In reply to Morris Tseng [:mtseng] from comment #3)
> Comment on attachment 8569593 [details] [diff] [review]
> Ignore events if there's no SelectionCarets elements. (v1)
> 
> Looks good. Do we need same check in TouchCaret?

TouchCaret is fine. It's protected by IsDisplayable() at the beginning of HandleEvent().

[1] http://hg.mozilla.org/mozilla-central/annotate/9b077c6f3d02/layout/base/TouchCaret.cpp#l594
Attachment #8569593 - Flags: review?(ehsan)
Attachment #8569593 - Flags: review?(ehsan) → review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/b1f4b4120e21
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox39: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
You need to log in before you can comment on or make changes to this bug.