Closed
Bug 240404
Opened 21 years ago
Closed 21 years ago
Site context menu broken
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla1.8alpha1
People
(Reporter: mcsmurf, Assigned: brendan)
References
Details
(Keywords: regression)
Attachments
(2 files)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
shaver
:
review+
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Updated•21 years ago
|
Flags: blocking1.8a?
Comment 3•21 years ago
|
||
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)
Reporter | ||
Comment 6•21 years ago
|
||
(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.
Comment 8•21 years ago
|
||
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...
Reporter | ||
Comment 9•21 years ago
|
||
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.
Comment 10•21 years ago
|
||
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
Assignee | ||
Comment 11•21 years ago
|
||
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
Reporter | ||
Comment 12•21 years ago
|
||
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?
Comment 13•21 years ago
|
||
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.
Reporter | ||
Comment 14•21 years ago
|
||
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.
Comment 15•21 years ago
|
||
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).
Assignee | ||
Comment 16•21 years ago
|
||
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
Assignee | ||
Comment 17•21 years ago
|
||
BTW, jrgm's FastLoad file dumper perl script is at bug 194614 attachment 115604 [details].
/be
Comment 18•21 years ago
|
||
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
Assignee | ||
Comment 19•21 years ago
|
||
I used
gcc version 3.3.2 20031022 (Red Hat Linux 3.3.2-1)
rkaa: what gcc version are you using?
/be
Comment 20•21 years ago
|
||
gcc-3.2.2-5 (RH9)
Comment 21•21 years ago
|
||
oops! The menus are missing again. They were there after a fresh build, but now
gone.
Assignee | ||
Comment 22•21 years ago
|
||
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 23•21 years ago
|
||
comment #5 still stands, i believe. I'm rebuilding that tree again (got
sidetracked by the new Gtk2 requirement for xft)
Assignee | ||
Comment 24•21 years ago
|
||
comment 5 doesn't isolate the js/src changes. I'm eagerly awaiting bad news!
/be
Reporter | ||
Comment 25•21 years ago
|
||
(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:
Comment 26•21 years ago
|
||
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].
Reporter | ||
Comment 27•21 years ago
|
||
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.
Comment 28•21 years ago
|
||
Might be completely irrelvant but I'm seeing lots of "reference to undefined
property navigator.platform.indexOf" strict JavaScript warnings...
Comment 29•21 years ago
|
||
> SeachStr at end of function:
It it actually empty? Or does it contain whitespace? What's SearchStr.length?
Reporter | ||
Comment 30•21 years ago
|
||
(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.
Comment 31•21 years ago
|
||
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)?
Reporter | ||
Comment 32•21 years ago
|
||
(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.
Assignee | ||
Comment 33•21 years ago
|
||
Please confirm that this fixes things. It eliminates those navigator.platform
strict warnings about indexOf being undefined.
/be
Comment 34•21 years ago
|
||
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
Assignee | ||
Comment 35•21 years ago
|
||
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
Comment 36•21 years ago
|
||
*** Bug 240530 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 37•21 years ago
|
||
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
Comment 38•21 years ago
|
||
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.
Assignee | ||
Comment 39•21 years ago
|
||
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
Comment 40•21 years ago
|
||
*** Bug 240494 has been marked as a duplicate of this bug. ***
Comment 41•21 years ago
|
||
*** Bug 240613 has been marked as a duplicate of this bug. ***
Comment 42•21 years ago
|
||
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.
Comment 43•21 years ago
|
||
*** Bug 240605 has been marked as a duplicate of this bug. ***
Comment 44•21 years ago
|
||
*** Bug 240660 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 45•21 years ago
|
||
Finally found some time to debug and think about this.
/be
Assignee | ||
Updated•21 years ago
|
Attachment #146653 -
Flags: review?(shaver)
Comment on attachment 146653 [details] [diff] [review]
duh
r=shaver
Attachment #146653 -
Flags: review?(shaver) → review+
Assignee | ||
Comment 47•21 years ago
|
||
Fixed, thanks to everyone for helping.
/be
Status: ASSIGNED → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Reporter | ||
Updated•21 years ago
|
Flags: blocking1.8a?
You need to log in
before you can comment on or make changes to this bug.
Description
•