Closed Bug 240404 Opened 21 years ago Closed 21 years ago

Site context menu broken

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
major

Tracking

()

RESOLVED FIXED
mozilla1.8alpha1

People

(Reporter: mcsmurf, Assigned: brendan)

References

Details

(Keywords: regression)

Attachments

(2 files)

With a current cvs trunk build the site context menu is broken. To reproduce just go to a internet site and click right. The normal menu doesn't appear, instead the menu appears as if some text was selected (Copy, Select All, Web Search For " ", View Selection Source). This regressed somewhere after opening of the tree for 1.8a development.
also, watch the JS console when doing a: Web Search for " "
Flags: blocking1.8a?
Also happens with a Linux build from today.
OS: Windows 2000 → All
My bets are on bug 237566, bug 235090 or bug 165201. This last one seems especially likely, since the nsContextMenu code uses those to filter the selection string.... See http://lxr.mozilla.org/seamonkey/source/mail/base/content/nsContextMenu.js#770 and http://lxr.mozilla.org/seamonkey/source/mail/base/content/nsContextMenu.js#749 for what should be the relevant nsContextMenu code.
Bug 237566 seems unlikly. That patch should only affect callers of replaceChild which isn't used by the contextmenu code (and hardly anywhere else in moz). If I screwed up it also could have affected rendering code, but that doesn't seem to be the issue here.
built locally with everything checked in at 2004-04-12 18:25 backed out, and context menu is OK again. (Linux)
(In reply to comment #5) > built locally with everything checked in at 2004-04-12 18:25 backed out, and > context menu is OK again. (Linux) That would be Bug 169559 then.
Seen from here, 30 files were modified at 18:25. The checkin description mentions bug 169559, bug 165201 and bug 206599.
Or 165201, as I pointed out initially (I still think it's more likely). Could someone with a build that's broken see what the nsContextMenu code is getting after each step? The 2004-04-13-08 nightly I just downloaded from ftp.mozilla.org is NOT showing this bug, so I can't exactly debug it from here...
One thing that i noticed: When i created a new profile the context menu worked fine. But then when i closed Mozilla and started with the same profile again, the menu was broken.
Hrm... sounds like a fastload issue, maybe? Again, some info on what's actually going on (what the string values are in that JS) would be the best way to move toward a fix.
Assignee: guifeatures → brendan
Quitting, removing the FastLoad file (XUL.mfl on windows, XUL.mfasl on Unix, "XUL FastLoad File" on Mac), and restarting in the same profile would help prove the problem was FastLoad and not anything else that sticks around in the profile. You can also set the nglayout.debug.disable_xul_fastload pref via about:config. I wonder whether I botched something in the jsxdrapi.c change that is hard for me to see. bz, you willing to look with fresh eyes? /be
I would like to take a look, but Venkman is also broken with this. When i delete XUL.mfl, the menu appears again and Venkman works again until restart. Should i attach the broken XUL.mfl here?
That part looks fine to me.... though if someone can test backing out just that one change (it's independent of the rest of the changes in question), that would be great.
Ok just uploaded it to http://mitglied.lycos.de/mcsmurf/xul.zip This is a xul.mfl from a complete new profile, i only started Mozilla, closed it, started it again (saw the wrong context menu) and closed it again.
Why do you need Venkman? Open up nsContextMenu.js, add some dump() calls.... if you use a decent text editor it can do this entirely within the jar; otherwise you may need to unjar and rejar (unzip and rezip, really).
I tested my JS-only, JS API and JS ABI compatible changes in an out of date tree, and I don't see any problems, assuming I'm reading comment 0 correctly. Could someone who can reproduce try backing out my whole js/src change and restesting, with no other changes? /be
BTW, jrgm's FastLoad file dumper perl script is at bug 194614 attachment 115604 [details]. /be
to comment 13: backed out only jsxdrapi.c from a broken build, and menus are OK again: cvs update -j1.24 -j1.23 mozilla/js/src/jsxdrapi.c
I used gcc version 3.3.2 20031022 (Red Hat Linux 3.3.2-1) rkaa: what gcc version are you using? /be
gcc-3.2.2-5 (RH9)
oops! The menus are missing again. They were there after a fresh build, but now gone.
Whew! That's good, because the jsxdrapi.c change looks good to me, bz, bryner, and dbaron (I predict -- he hasn't gotten back to me on it). Try backing out my whole patch. If that doesn't fix it, it wasn't me. Were the nsISeekableStream 64-bit changes part of today's build? /be
comment #5 still stands, i believe. I'm rebuilding that tree again (got sidetracked by the new Gtk2 requirement for xft)
comment 5 doesn't isolate the js/src changes. I'm eagerly awaiting bad news! /be
(In reply to comment #3) > My bets are on bug 237566, bug 235090 or bug 165201. This last one seems > especially likely, since the nsContextMenu code uses those to filter the > selection string.... > > See > http://lxr.mozilla.org/seamonkey/source/mail/base/content/nsContextMenu.js#770 > and > http://lxr.mozilla.org/seamonkey/source/mail/base/content/nsContextMenu.js#749 > for what should be the relevant nsContextMenu code. Don't know if it is still relevant, but this comes out for 2 vars when selecting some text (the menu is still broken, doesn't include the selected text in Web Search for " "): SeachStr: letting the light in Selection: SeachStr: Leave as NEW Selection:
I looked at http://mitglied.lycos.de/mcsmurf/xul.zip with my silly script. It seems well-formed for the rules that that script checks. [By the way, I couldn't actually download that zip file with firefox 0.8 on mac os x; I resorted to command line tools to get a copy].
Ok, some add to my previous comment, the function searchSelected seems to do something strang to searchStr: SeachStr at begin of function: letting the light in SeachStr at end of function: Selection: It seems to cut the whole String off.
Might be completely irrelvant but I'm seeing lots of "reference to undefined property navigator.platform.indexOf" strict JavaScript warnings...
> SeachStr at end of function: It it actually empty? Or does it contain whitespace? What's SearchStr.length?
(In reply to comment #29) > > SeachStr at end of function: > > It it actually empty? Or does it contain whitespace? What's SearchStr.length? Length is 1.
Well, that would be the cause of this bug. Could you trace down which particular regexp in that function is breaking (check the value after every regexp)?
(In reply to comment #31) > Well, that would be the cause of this bug. Could you trace down which > particular regexp in that function is breaking (check the value after every regexp)? This regxp searchStr = searchStr.replace(/\s+/g, " "); breaks it.
Please confirm that this fixes things. It eliminates those navigator.platform strict warnings about indexOf being undefined. /be
the patch cures the context menu. But simply opening the js console still spawn two of these warnings: Warning: reference to undefined property navigator.platform.indexOf Source File: chrome://global/content/bindings/scrollbar.xml Line: 28
I'll have a fix today, or back out the whole patch landed Monday. Probably the former, but whatever it takes. Sorry for the trouble. /be
Status: NEW → ASSIGNED
Component: XP Apps: GUI Features → JavaScript Engine
Hardware: PC → All
Target Milestone: --- → mozilla1.8alpha
*** Bug 240530 has been marked as a duplicate of this bug. ***
rkaa, does a clean profile (or better, just removing XUL.mfasl) eliminate those navigator.platform.indexOf warnings? If not, then those are a separate bug. /be
Warning: reference to undefined property document.write Source File: http://bugzilla.mozilla.org/show_bug.cgi?id=240404 Line: 111 :) No, deleting XUL.mfasl didn't do a thing - even more warnings, and two more on each and every pageload it seems, so that is something else then.
The navigator.platform.indexOf complaint was a bogus strict warning I caused by making FindConstructor in jsobj.c call OBJ_GET_PROPERTY instead of looking up the class name and only if it exists, getting its native slot value. The way things used to work, global objects were required to be native. I fixed that, but the OBJ_GET_PROPERTY call when nested resolving a class name such as 'String' (for the navigator.platform string value to be wrapped in an object from which to get the indexOf method) would hit the strict warning for accessing an undefined property. I made FindConstructor avoid the get call by checking cx->resolvingTable. I also checked in the hackaround for jsscan.c. Tomorrow's builds should work as before. If they do not, please let me know. Keeping this bug open so I can find the elusive regexp problem. /be
*** Bug 240494 has been marked as a duplicate of this bug. ***
*** Bug 240613 has been marked as a duplicate of this bug. ***
This seem to also be breaking ChatZilla on cached starts (i.e. when XUL.mfl exists) - it looks like the string-bundle isn't importing it's contents properly. The good news is that attachment 146112 [details] [diff] [review] seems to fix it here.
*** Bug 240605 has been marked as a duplicate of this bug. ***
*** Bug 240660 has been marked as a duplicate of this bug. ***
Attached patch duh (deleted) — Splinter Review
Finally found some time to debug and think about this. /be
Attachment #146653 - Flags: review?(shaver)
Fixed, thanks to everyone for helping. /be
Status: ASSIGNED → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Flags: blocking1.8a?
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: