Closed Bug 94990 Opened 24 years ago Closed 24 years ago

Several contextual menu items broken (Open in New Window, others)

Categories

(SeaMonkey :: UI Design, defect)

Product:
SeaMonkey
Component:
UI Design
Type:
defect
Priority:
Not set
Severity:
blocker

Tracking

(Not tracked)

Status:
VERIFIED FIXED

People

(Reporter: majick, Assigned: hyatt)

References

Details

(Keywords: regression, smoketest)

Attachments

(1 file)

workaround (renames old contextMenu to _contextMenu)
10.67 KB, patch
Details | Diff | Splinter Review
WinME 2001081208 Context menus don't show up when right-clicking anywhere in the browser pane. Links, text, images, or empty space: it just doesn't matter. For contrast, a context menu does function in, for example, the address bar.
linux too.
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows ME → All
Yep. Neither does certain KB Nav functions (alt+left, etc). This only appears on the 8/12 builds this morning it seems.
URL: any
Keywords: regression
*** Bug 94996 has been marked as a duplicate of this bug. ***
Now that I've got a workaround for bug 94992, here's a little something from the JS console: Error: contextMenu has no properties Source File: chrome://cookie/content/cookieContextOverlay.xul Line: 94
All/All -- seeing this on Mac OS X, too.
Hardware: PC → All
Confirmed on win98 with build 2001081208
CCing hyatt because I think his last checkin might have regressed this. But I may be wrong.
problem is in a cookies file. --> cookies
Assignee: trudelle → morse
Component: XP Toolkit/Widgets → Cookies
QA Contact: aegis → tever
Backing out hyatt solves the problem for me. This is on Linux. Commands used: cvs update -j1.352 -j1.351 mozilla/content/xul/content/src/nsXULElement.cpp cvs update -j1.6 -j1.5 mozilla/dom/public/idl/xul/nsIDOMXULElement.idl cvs update -j1.351 -j1.350 mozilla/content/xul/content/src/nsXULElement.cpp cvs update -j1.5 -j1.4 mozilla/dom/public/idl/xul/nsIDOMXULElement.idl cvs update -j1.4 -j1.3 mozilla/dom/public/idl/xul/nsIDOMXULElement.idl cvs update -j1.350 -j1.349 mozilla/content/xul/content/src/nsXULElement.cpp cvs update -j1.73 -j1.72 mozilla/content/shared/public/nsXULAtomList.h
--> hyatt
Assignee: morse → hyatt
*** Bug 95031 has been marked as a duplicate of this bug. ***
Shouldn't this be a blocker? Several smoketests requiring the use of context menu. Marking smoketest and nominating for mozilla0.9.4.
Keywords: mozilla0.9.4, smoketest
I checked in an interim workaround that gets the context menus appearing again, but there seems to be a screwy scope chain in the DOM/JS here. jst, over the weekend I added a contextMenu property to XUL elts. Now it also happens that there's some JS code that uses a global contextMenu variable. Look in contentContextOverlay.xul. In an event handler, contextMenu is assigned into, e.g., onpopupshowing="contextMenu = new nsContextMenu(this);" That assignment is assigning into the XUL element contextMenu property! XULElement's SetContextMenu method is being called! It's as if I'd written: onpopupshowing="this.contextMenu = ..." I believe this is wrong. In the above case, the global variable should have been assigned into. Rather than backing myself out, I'd like to get to the bottom of the real bug. There will be other context menu glitches until this is fixed.
Assignee: hyatt → jst
Workaround checked in. Removing smoketest keyword.
Keywords: smoketest
The workaround does not fully work. I am able (fresh CVS build on WinME) to open the context menu but "Open in new window" does not work (nothing happens). More precisely it works from the mail window but not a browser window. So it may be still a smoketest blocker.
*** Bug 95038 has been marked as a duplicate of this bug. ***
Since DOM level 0 daze, event handlers have included their 'this' param (and its ancestors linked by __parent__) in their scopechains. I don't see how we can break compatibility. If you want to set a global contextMenu variable in such an event handler, use 'window.contextMenu = ...' (sorry if that was your workaround and you know this already). In a JS function, it's best to qualify assignments to global variables if there is any doubt about the function's scope chain. eval calls, with statements and event handlers make for unknown property contents of objects on the scope chain, so qualification is the only way to go. /be
*** Bug 95078 has been marked as a duplicate of this bug. ***
As Right-Click -> Open New Window and Right-Click -> Edit Page In Composer do not work (see duped bug 95078) this may become a mostfreq bug in a few days. I urge giving it a very high priority. It would be a very good candidate for a "mostannoying" keyword, we lack so much.
is this still a cookies problem?
*** Bug 95097 has been marked as a duplicate of this bug. ***
Taking summary from bug 95097 to reflect new problem and avoid some dupes.
Summary: Context menu does not appear in browser pane → Several contextual menu items broken (Open in New Window, others)
I'm stunned that it works this way. In XBL event handlers you have to qualify properties with the "this" keyword. I can only assume that I am neglecting to set up some sort of implicit relationship in XBL, and that led to my confusion about what real event handlers do. Ok, I will take it back and rename the variable.
Assignee: jst → hyatt
If XBL makes the parent of each function object it creates the 'this' parameter, then that function, when activated, will have as its scope chain its own activation object, followed by the 'this' object, followed by the parent of the 'this' object, etc., up to the global object. It looks like XBL compiles function objects for <method> with the class object as their parent. Same for getters and setters. Over in content/xbl/src/ nsXBLPrototypeHandler.cpp, I see nsIScriptContext's CompileEventHandler and BindCompiledEventHandler used to compile an event handler function as a method of a script object, ensuring that the handler's parent is the object that will receive the event. An nsIJSEventListener is then created with the same receiver object, so it looks like XBL event handlers will have the same scope chain as DOM-defined ones. BTW, XBL could avoid some wasted cycle by passing PR_FALSE instead of PR_TRUE as the fourth argument to boundContext->CompileEventHandler in nsXBLPrototypeHandler::ExecuteHandler, and therefore not call boundContext->BindCompiledEventHandler at all. If you are compiling an unshared event handler, it'll be bound to aTarget (the nominal receiver) by CompileEventHandler, and you don't need to BindCompiledEventHandler. (But this all shows what we knew: XBL should compile event handlers once per binding, and not for every handler dispatch. Is mscott fixing that with brutal sharing?) /be
Andreas, why ugly up the global name with a leading underscore? It suffices to use window.contextMenu in the event handler (no one uses window for any name other than the global object's property that names itself). If we're worried about confusion down the road, let's have two underscore-free, distinct names. But window.contextMenu is always clearer than any synonym, I think. /be
My patch converts the name to gContextMenu, since that is an established FE convention for navigator.
*** Bug 95137 has been marked as a duplicate of this bug. ***
"save as" and "save link as" are dead as well.
This should be a smoketest blocker. (Asa and I agree.)
Severity: major → blocker
Keywords: smoketest
Ah, gContextMenu worksforme -- but where's that patch? /be
Fix checked in.
Status: NEW → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Yo, Dave. You left the rev 1.8 try/catch in cookieContextOverlay.xul. No need to leave that behind. (Holy nit-picking, eh).
*** Bug 95219 has been marked as a duplicate of this bug. ***
i'm using build 2001081303 on WinNT, the context menu appears but not all are working some of the things that don't work - Open in New Window - Edit Link in Composer - View Page Info - Save as... - Save Link as... - Properties the context menus in the mail reader works, it seems that the problem is isolated in the browser, I'm not that sure though
2001081303 is about 12 hours before the full fix for this was checked in. in 2001081412 builds, this is fixed. Browser context menus take their respective actions; no js errors.
Status: RESOLVED → VERIFIED
There are still instances of |contextMenu| at http://lxr.mozilla.org/seamonkey/source/xpfe/communicator/resources/content/nsContextMenu.js#719 Shouldn't they be |gContextMenu| too?
Sorry, they are strings. Only cosmetic problem.
*** Bug 94720 has been marked as a duplicate of this bug. ***
-> xp apps, some dupes are there...
Component: Cookies → XP Apps
Product: Core → Mozilla Application Suite
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: