Closed Bug 797297 Opened 10 years ago Closed 10 years ago

Contextmenu fired on text nodes crashes

Categories

(Core :: DOM: Core & HTML, defect)

Product:
Core
Component:
DOM: Core & HTML
Platform:
x86
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla19
Project Flags:
blocking-basecamp +
Tracking Flags:
Tracking Status
firefox18 --- fixed
firefox19 --- fixed

People

(Reporter: daleharvey, Assigned: daleharvey)

Details

Attachments

(1 file, 1 obsolete file)

Bug 797297 - Remove hasAttribute check from outer loop. r=jlebar
2.17 KB, patch
justin.lebar+bug
: review+
Details | Diff | Splinter Review 
When a contextmenu gets triggered on a text node within a mozbrowser the parent nodes arent collected as the textnode fails the hasAttribute test
Assignee: nobody → dale
Status: NEW → ASSIGNED
Attachment #667398 - Flags: feedback?(justin.lebar+bug)
Comment on attachment 667398 [details] [diff] [review]
Remove hasAttribute check from outer loop

A test would be appreciated, but this looks sane to me.
Attachment #667398 - Flags: feedback?(justin.lebar+bug) → feedback+
Changed one of the existing tests to hit the case that used to cause a crash
Attachment #667398 - Attachment is obsolete: true
Attachment #671167 - Flags: review?(justin.lebar+bug)
Attachment #671167 - Flags: review?(justin.lebar+bug) → review+
To nit about the bug summary: We don't usually call JS throwing an exception a "crash".  Unless somehow a process is dying as a result of this?

https://tbpl.mozilla.org/?tree=Try&rev=3ce586b2448d
Try run for 3ce586b2448d is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=3ce586b2448d
Results (out of 13 total builds):
    success: 13
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jlebar@mozilla.com-3ce586b2448d
Ah no, still thinking in erlang sometimes :) the event just throws an error and contextmenu isnt triggered, there was not full crash or anything. Looks like the try build is good though, Cheers, should have remember to push to try myself
> Cheers, should have remember to push to try myself

I normally wouldn't even push something like this to tryserver, except I've been bitten a few too many times recently.  :shrug:

https://hg.mozilla.org/integration/mozilla-inbound/rev/574ec568513b
https://hg.mozilla.org/mozilla-central/rev/574ec568513b
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
Justin should this be flagged for aurora checkin? since the original context menu is blocking+
blocking-basecamp: --- → ?
Whiteboard: [needs-checkin-aurora]
blocking-basecamp: ? → +
You need to log in before you can comment on or make changes to this bug.