Closed Bug 94990 Opened 23 years ago Closed 23 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: 23 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: