Closed Bug 240404 Opened 16 years ago Closed 16 years ago

Site context menu broken

Categories

(Core :: JavaScript Engine, defect, major)

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 duhSplinter 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: 16 years ago
Resolution: --- → FIXED
Flags: blocking1.8a?
You need to log in before you can comment on or make changes to this bug.