Last Comment Bug 672258 - Wrong context menu can be shown if two different windowtypes have the same contextmenu id and xul cache is enabled
: Wrong context menu can be shown if two different windowtypes have the same co...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: XUL (show other bugs)
: Trunk
: All All
: -- major (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
:
:
Mentors:
: 674092 (view as bug list)
Depends on: 773945
Blocks: 674246
  Show dependency treegraph
 
Reported: 2011-07-18 10:15 PDT by Ian Neal
Modified: 2012-07-19 12:46 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
sample (1013 bytes, text/plain)
2011-07-20 08:47 PDT, Neil Deakin (away until Oct 3)
no flags Details
Bad Context Menu (151.43 KB, image/png)
2011-08-16 07:06 PDT, Wm Fay
no flags Details

Description Ian Neal 2011-07-18 10:15:53 PDT
Steps to reproduce:
1/ Have two XUL windows (type A and type B) with different window types but both have xul elements with contextmenus that have the same id
2/ Open first window of type A and second window of type B.
3/ Bring up context menu for relevant element in type B window.
4/ Bring up context menu for relevant element in type A window.

Actual results:
Both context menus brought up are for type B window.

Expected results:
Context menus should be different depending on whether element is in type A or Type B window.
Comment 1 Neil Deakin (away until Oct 3) 2011-07-18 11:30:15 PDT
Testcase?
Comment 2 Ian Neal 2011-07-18 13:19:26 PDT
(In reply to comment #1)
> Testcase?

There are some steps in comment 6 bug 671192 which I can reproduce but not sure how easy it will be to get a reduced testcase from that. I did create something very simple but probably too simple as it did not show the issue, so I will keep trying.
Comment 3 Neil Deakin (away until Oct 3) 2011-07-19 07:15:44 PDT
This is a bit unclear. This bug talks about two windows, but comment 13 in bug 671192 describes two overlays with conflicting elements, which sounds more like a problem with the code you're using.
Comment 4 Ian Neal 2011-07-19 07:38:57 PDT
(In reply to comment #3)
> This is a bit unclear. This bug talks about two windows, but comment 13 in
> bug 671192 describes two overlays with conflicting elements, which sounds
> more like a problem with the code you're using.

There are two windows, each with the same overlay which provides the context menu, but then one of those windows is overlayed again to add extra functionality for that window's context menu.

As was said in comment 13, yes they have the same context menu id (otherwise you would not be able to use the same overlay for both windows), but that should not be causing this issue.
Comment 5 Ian Neal 2011-07-20 07:34:52 PDT
Some further information discovered by NeilAway, this only happens if the xul cache is enabled. There is no problem if the xul cache is disabled.
Comment 6 neil@parkwaycc.co.uk 2011-07-20 08:27:14 PDT
Hopefully this will help someone make a minimal test case. (The original issue relates to the onpopupshowing handler but onclick should be easier to test.)

doc1.xul: contains an element, let's call it "X".
doc1.xul uses ovl1.xul as an overlay.
ovl1.xul: appends element "Y" to element "X".
          It also gives Y an inline event handler e.g. onclick="alert(1);"
doc2.xul: also contains element "X".
doc2.xul uses both ovl1.xul and ovl2.xul as overlays.
ovl2.xul overwrites element Y's inline event handler onclick="alert(2);"

Now if you open the documents and examine their contents in DOM inspector, everything looks hunky-dory - element X in onclick="alert(1)" in doc1 and onclick="alert(2)" in doc2. But the weird thing is, whichever one you click first, that alert will appear, even if you then click the other document!
Comment 7 Neil Deakin (away until Oct 3) 2011-07-20 08:47:11 PDT
Created attachment 547108 [details]
sample

Neil, a scenario something like this? I can't reproduce with this test.
Comment 8 Philip Chee 2011-07-26 12:58:05 PDT
*** Bug 674092 has been marked as a duplicate of this bug. ***
Comment 9 neil@parkwaycc.co.uk 2011-07-26 14:40:27 PDT
OK, so I dug around in nsScriptEventHandlerOwnerTearoff which is what attempts to share the compiled code between multiple instances of the same prototype element. Unfortunately it's trying to share code even though one of the attributes has changed. Normally SetAttribute causes something to change so that we don't try to share code but as I don't know where that magic happens, I can only assume that overlays don't trigger it.
Comment 10 neil@parkwaycc.co.uk 2011-07-26 14:48:16 PDT
(In reply to comment #7)
> Neil, a scenario something like this? I can't reproduce with this test.
No, the element must be created by overlay1 and then its event handler must be overwritten in document2 by overlay2.
Comment 11 neil@parkwaycc.co.uk 2011-07-26 14:49:13 PDT
(In fact defining the element in the document changes its prototype thus working around the problem.)
Comment 12 Wm Fay 2011-08-16 07:06:55 PDT
Created attachment 553459 [details]
Bad Context Menu

I've never seen a context menu this long. It either is a merged list or an outright bug.
^ (at least 56 entries)
Reject popup windows from this website
Allow popup windows from this website
--------------------------------------
(no Spelling Suggestions)
--------------------------------------
Add to Dictionary
Ignore Word
--------------------------------------
Open Link in New Tab
Open Link in New Window
--------------------------------------
Bookmark This Link
Send This Link...
Copy Email Address
Copy Link Location
--------------------------------------
Play
Pause
Mute
Unmute
Show Media Controls
Hide Media Controls
Full Screen
--------------------------------------
Fit Image to Window
ReloadImage
View Image

Copy Image
View Video
Copy Video Location
Copy Audio Location
--------------------------------------
Send Image...
Set As Wallpaper
Save Video As...
Send Video...
Save Audio As...
Send Audio...
--------------------------------------
Back
Forward
Reload
Stop
--------------------------------------
Bookmark This Page
Send This Page...
--------------------------------------
View Background Image
Undo
Redo
--------------------------------------
Cut
Copy
Paste
Delete
--------------------------------------
Select All
--------------------------------------
Add a Keyword for this Search...
--------------------------------------
View Selection Source
View MathML Source
View Page Source
View Page Source
Properties
--------------------------------------
This Frame >
--------------------------------------
Check Spelling
Download More Dictionaries
Languages >
Comment 13 Wm Fay 2011-08-16 07:39:38 PDT
Personally I would never have signed off on releasing this re-write of Seamonkey without much more extensive testing. It is many times more complex than Firefox. There are too many alternative ways of getting into a certain set of code.

Like this. I recall that I came in from Mail and I think the view options were not the way I had last set them. Message pane was on and the folder list was off. I even think there were more than one tab active in mail, but I don't pay attention to that because I don't use tabs for mail.

I am suspicious that Seamonkey was attempting to recover from a failure, but I don't recall any messages or dialogs to that affect.

I am concerned that this is being precipitated by Seamonkey not fully exiting after it has been closed down. I've seen that at least once, because I think it prevents re-loading the program.

Part of my problem is that I have come to rely so heavily on Seamonkey. I use it under at least 3 versions of Fedora plus XP, so I don't pay a lot of attention to minor discrepancies in display or behavior.
Comment 14 neil@parkwaycc.co.uk 2012-07-19 12:46:04 PDT
Fixed by bug 773945.

Note You need to log in before you can comment on or make changes to this bug.