Closed Bug 120934 Opened 23 years ago Closed 23 years ago

Major Navigation Problems on Olympics Website

Categories

(Core :: DOM: Events, defect)

Product:
Core
Component:
DOM: Events
Platform:
x86
All
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

()

Status:
VERIFIED FIXED

People

(Reporter: markushuebner, Assigned: roc)

References

(
URL
)

Details

(Keywords: regression, testcase, top100)

Attachments

(2 files)

not so simplified testcase
16.38 KB, text/html
Details
Fix
693 bytes, patch
kmcclusk
: review+
waterson
: superreview+
Details | Diff | Splinter Review 
using build 2002011803 on winxp.
The last menu items on the DHTML navigation cannot be reached (doesn't get 
activated).

Under "Spectator Info" no menu item at all can be gone to!
This is really important since this website will probably be the most
visited website in the next couple of weeks.
Keywords: qawanted, top100
Testing 0.9.6 the menu works fine. 0.9.7 is already having this bug.
Adding keyword "regression".
Keywords: regression
0.9.6 is fine, 0.9.7 shows problems, ergo, regression.

I have no idea where to send this.
Keywords: regression
OS: Windows 2000 → All
Keywords: regression
Could you make a reduced testcase?
Keywords: mozilla0.9.9
attach is a not so simplified testcase I've just extracted the menubar from the
page and added some window.dump on the mouseover mouseout listeners. on win32
run moz with "mozilla.exe -console" to see the dump
This looks *very* related to the problems seen with the menus on
http://www.msnbc.com. I've seen bugs filed on those menus too, but I can't find
the bugs right now. I don't recall anyone tracking down the problems yet though.

Looks like mouse events are leaking through the menu's into content underneeth
the menus, or something like that.
Keywords: mozilla0.9.8, testcase
bug 118689

the odd thing is that it only affect some menus and menuitems, I can't seem to
see anything special about those menus/menuitems except that they are usually
the last few menus/menuitems.
Component: Browser-General → DOM Events
Summary: Navigation Problems on Olympics Website → Major Navigation Problems on Olympics Website
117312 is my bug about msnbc menus not working, it originally seemd like they
were blocking mozilla, but that is not true.
Assignee: asa → joki
QA Contact: doronr → vladimire
Not sure if it matters, but I get a bunch of JS Console messages from basic's
testcase.

As the page loads:

Warning: function nm_bc does not always return a value
Source File: http://bugzilla.mozilla.org/showattachment.cgi?attach_id=65803
Line: 109
Source Code:
}

Warning: variable sU hides argument
Source File: http://bugzilla.mozilla.org/showattachment.cgi?attach_id=65803
Line: 242, Column: 7
Source Code:
					s,sU,sT;

(this next one 13 times -- event handler bug certainly)
Warning: reference to undefined property oUsn.ie
Source File: http://bugzilla.mozilla.org/showattachment.cgi?attach_id=65803
Line: 448

Warning: assignment to undeclared variable o
Source File: http://bugzilla.mozilla.org/showattachment.cgi?attach_id=65803
Line: 74

Warning: assignment to undeclared variable asUrl
Source File: http://bugzilla.mozilla.org/showattachment.cgi?attach_id=65803
Line: 43

Mousing over a top menu:
(13 times, again an event-related strict warning)
Warning: reference to undefined property document.getElementById("nm_c" + (nD +
1)).nm_aOff
Source File: http://bugzilla.mozilla.org/showattachment.cgi?attach_id=65803
Line: 332

Mousing over a sub-menu, the same 13 warnings immediately above, plus:
(13 times, again an event-related strict warning)
Warning: reference to undefined property o.nm_bDrp
Source File: http://bugzilla.mozilla.org/showattachment.cgi?attach_id=65803
Line: 273

Mousing over an entry in the sub-menu:
(13 times, again an event-related strict warning)
Warning: reference to undefined property nm_aDef[6]
Source File: http://bugzilla.mozilla.org/showattachment.cgi?attach_id=65803
Line: 329
FWIW, the testcase references a large number of defined strings at
http://www.saltlake2002.com/news/osm.js .  Looks like information the menu uses.
I don't know if you guys keep them, but we have OS/2 nightlies back for a year.

I can probably narrow down the dates between which this started breaking.

I'll take a look at it later today.
Ok, this seems to be related to settimeout(). Remove all calls to settimeout in
the  attached testcase did it for me.
mkaply: good builds to test would be around pavlov's timer landing around 12-16.
This actually broke between November 30 2001 at 4PM PST and Dec 1 2001 at 8AM 
PST 

Here's the bonsai link:

http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&bra
nchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&d
ate=explicit&mindate=11%2F30%2F2001+16%3A00&maxdate=12%2F01%2F2001+8%3A00&cvsroo
t=%2Fcvsroot

Note that the times should be pretty accurate, but we might want to look a few 
hours or so extra in each direction.
Thanks for tracking down what day this broke on! If I'd haveto guess which of
those checkins broke this, my first guess would be Robert O'Callahan's
(roc+moz@cs.cmu.edu) view manager cleanup changes (bug 73382). Could someone
verify that? Robert, would you be able to investigate this? This breaks at least
two *large* sites and should be fixed ASAP (ideally for mozilla0.9.8).
adding myself to cc ...
I'll take a look at it.
roc, you wanna reassign the bug to yourself?

/be
OK.

If anyone has a testcase that wasn't a zillion lines of Javascript, that'd be
great...
Assignee: joki → roc+moz
My patch in bug 13213 seems to fix this!
I'm not sure WHY my 13213 patch fixes this; these menus don't seem to have the
top/left problems that 13213 is addressing, and if they did, then they would
NEVER have worked in Mozilla. I'll try to narrow it down.

In the meantime if someone wants to help out they could either create a
simplified test case that doesn't involve Javascript, or try different parts of
the 13213 patch to see what works, or else just verify that the 13213 patch
fixes the problem for them.
OK, here's my guess for a fix: change

vm->ResizeView(aView, *aCombinedArea);

back to

vm->ResizeView(aView, aCombinedArea->XMost(), aCombinedArea->YMost());

in layout/html/base/src/nsContainerFrame.cpp

I'll test this out.
Attached patch Fix — — Splinter Review 
OK, THIS is actually what we need to do.

Testing underway
OK, the fix works!

Here's what's happening: with the current code, aCombinedArea in
ContainerFrame::SyncFrameViewAfterReflow() *almost always* has an (x,y) of (0,0)
because it includes the area of the view's frame, which is always at (0,0) for
its view. But there is one situation where aCombinedArea can have nonzero (x,y):
when the view's frame is completely empty. Then aCombinedArea will just be the
union of the child frame bounds, which may not be at (0,0). In that case I
introduced a bug where the (x,y) was not accounted for so the view was not made
big enough. Oops!

If we can get this reviewed quickly we may still make 0.9.8.
Status: NEW → ASSIGNED
Keywords: review
roc, who've you asked to review this patch?  It looks ok to me, but I don't want
to jump anyone's claim.

/be
kmcclusk and attinasi are my usual r/sr buddies. I copied them on my email to
drivers.
Comment on attachment 65994 [details] [diff] [review]
Fix

Patch looks good to me.
r=kmcclusk@netscape.com
Attachment #65994 - Flags: review+
attinasi is not around for the next 2 weeks. You may want to try waterson for a sr.
Comment on attachment 65994 [details] [diff] [review]
Fix

sr=waterson
Attachment #65994 - Flags: superreview+
a=asa (on behalf of drivers)
Keywords: mozilla0.9.8, mozilla0.9.9 → mozilla0.9.8+
Marking fixed.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
V Linux 2002012306. All the menus now work.
verifying on 2002-01-29-03-trunk on windows 98
and 2002-01-28-08-trunk on linux RedHat
Status: RESOLVED → VERIFIED
Keywords: qawanted
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: