window closing before selecting item from context menu crashes Camino [@ nsCOMPtr<nsIDOMNode>::get_DerivedSafe][@ -[BrowserWindowController copyImageLocation:] ], etc.

RESOLVED FIXED

Status

--
critical
RESOLVED FIXED
10 years ago
10 years ago

People

(Reporter: hhaamu, Assigned: bugzilla-graveyard)

Tracking

({fixed1.8.1.22})

unspecified
All
Mac OS X
fixed1.8.1.22
Bug Flags:
camino1.6.7 +

Details

(URL)

Attachments

(5 attachments, 2 obsolete attachments)

(Reporter)

Description

10 years ago
User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en; rv:1.9.0.8pre) Gecko/2009022317 Camino/2.0b2
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en; rv:1.9.0.8pre) Gecko/2009022317 Camino/2.0b2

The URL opens a new browser window with image. The pop-up is designed to close at an onmousedown event. Right-clicking displays context menu, but triggers onmousedown event (which closes window).

Selecting an item from the (dangling) context menu crashes browser.

Reproducible: Always

Steps to Reproduce:
1. Click on the image to trigger pop-up
2. Right-click on image in the pop-up
3. [pop-up closes due to onmousedown event]
4. Select "Copy image location"
5. Crash
Actual Results:  
a short beachballing, and then crash


Related: bug 168065
(Reporter)

Comment 1

10 years ago
Created attachment 364751 [details]
testcase

Testcase. Can probably be simplified a bit.

Comment 2

10 years ago
Created attachment 364756 [details]
crash log

btw, this is not new to Camino 2.0b2, I can go back to the 20080915 nightly build. Will check later with older builds.

Minefield & Fx 3 don't crash (the context menu is not triggered).
This is probably a dupe of bug 342672 (and maybe bug 183586, which could have been reopened by later appshell changes).  It's certainly the same basic bug as those two and bug 168065.
(Assignee)

Comment 4

10 years ago
Can the code associated with window closure be modified to ensure that any context menus on that window are closed as well? That seems like it would fix this.

What we really need is a fix for bug 168065, though.
Since the other appshell+context menu bugs are still, in fact, fixed, confirming this.

Oddly enough, "Copy Image" does *not* trigger the crash.

As an alternate strategy to fixing bug 168065, can we ensure that all of our context menu items check to make sure the document/image/selection/whatever that they think they're operating on is still around?
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: window closing before context menu crashes browser → window closing before context menu crashes [@ nsCOMPtr<nsIDOMNode>::get_DerivedSafe][@ -[BrowserWindowController copyImageLocation:] ], etc.
Created attachment 365245 [details]
debug crash log

We get some Gecko at the top the thread in a debug build, fwiw.
Summary: window closing before context menu crashes [@ nsCOMPtr<nsIDOMNode>::get_DerivedSafe][@ -[BrowserWindowController copyImageLocation:] ], etc. → window closing before selecting item from context menu crashes Camino [@ nsCOMPtr<nsIDOMNode>::get_DerivedSafe][@ -[BrowserWindowController copyImageLocation:] ], etc.
(Assignee)

Comment 7

10 years ago
Created attachment 365298 [details]
somewhat better testcase

This testcase makes the image a link as well, so we get all the link CM items (which were also mostly crashing here).

I'm about to post a patch that implements comment 5 and fixes all the crashes.
Assignee: nobody → cl-bugs-new
Attachment #364751 - Attachment is obsolete: true
(Assignee)

Comment 8

10 years ago
Created attachment 365300 [details] [diff] [review]
fix v1.0

This implements comment 5. I can't figure out how to do comment 4, but these null-checks seem like a good idea anyway, even if the other bug gets fixed and/or someone figures out how to do comment 4.
Attachment #365300 - Flags: review?
(Assignee)

Updated

10 years ago
Status: NEW → ASSIGNED
Hardware: x86 → All
(Assignee)

Comment 9

10 years ago
This is probably an issue on branch as well, so do we want this for 1.6.7?
Flags: camino1.6.7?

Comment 10

10 years ago
(In reply to comment #9)
> This is probably an issue on branch as well, so do we want this for 1.6.7?
When I tested this with the patch in comment 0, I couldn't get 1.6.6 to crash. It does crash with the second testcase and choosing 'save image as…'.
Created attachment 365387 [details]
more extensive testcase

Besides "Copy Address" which I've tested to crash, what items from other context menus also need to be fixed?
Attachment #365298 - Attachment is obsolete: true
Attachment #365387 - Attachment is patch: false
Attachment #365387 - Attachment mime type: text/plain → application/xhtml+xml
Attachment #365387 - Attachment mime type: application/xhtml+xml → text/html
(Assignee)

Comment 12

10 years ago
With the patch in comment 8, I can't get a crash with any of the CMs that can be triggered with the new testcase from comment 11.

Updated

10 years ago
Attachment #365300 - Flags: review? → review?(trendyhendy2000)

Updated

10 years ago
Attachment #365300 - Flags: review?(trendyhendy2000) → review+
(Assignee)

Updated

10 years ago
Attachment #365300 - Flags: superreview?(mikepinkerton)
Created attachment 367163 [details] [diff] [review]
branch version

Thanks to some investigation from hendy, here's the branch version of the patch.
Comment on attachment 365300 [details] [diff] [review]
fix v1.0

sr=pink
Attachment #365300 - Flags: superreview?(mikepinkerton) → superreview+
Landed on cvs trunk and the MOZILLA_1_8_BRANCH for 1.6.7.
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Flags: camino1.6.7? → camino1.6.7+
Keywords: fixed1.8.1.22
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.