Closed
Bug 225732
Opened 21 years ago
Closed 21 years ago
Menu items don't activate properly (see comment 12)
Categories
(Core :: XUL, defect)
Core
XUL
Tracking
()
RESOLVED
FIXED
People
(Reporter: mozilla, Assigned: bzbarsky)
Details
(Keywords: regression)
Attachments
(2 files, 1 obsolete file)
6.39 KB,
patch
|
dbaron
:
review+
dbaron
:
superreview+
|
Details | Diff | Splinter Review |
6.69 KB,
patch
|
Details | Diff | Splinter Review |
2003111314 trunk (Tinderbox build) It appear that all of the menus are messed up in Mozilla. Menu items do not activate unless you mouseover the actual text of the menu item. This is contrary to how Mozilla used to behave, and contrary to how all other Windows applications behave. Menu items are supposed to activate any time the mouse is over the menu anywhere. This is also a regression.
Assignee | ||
Comment 1•21 years ago
|
||
> This is also a regression.
Any chance of determining the regression window?
Reporter | ||
Comment 2•21 years ago
|
||
Not at all possible. This is because there haven't been any Win32 builds since 10-28. I can tell you that it worked fine then, and that's it. If there is a repository of Tinderbox builds, I can look through them.
Assignee | ||
Comment 3•21 years ago
|
||
There is no such beast. Is this a problem with Firebird too? Are there Firebird builds from that time period? This bug needs a real owner, since hyatt is not likely to be fixing it and I'm not that owner (since I have no windows build, hence can't even tell what the bug is about based on the description...)
Keywords: helpwanted
Reporter | ||
Comment 4•21 years ago
|
||
There are no Firebird builds since 10-28 either. I have tried building from source, but then there are problems. See bug 225059.
Assignee | ||
Comment 5•21 years ago
|
||
Ugh. ccing some people who may have windows builds to hand. A two-week regression window is huge... :(
Comment 6•21 years ago
|
||
well... I don't have 2003111314 available, but 2003111209 (also tinderbox build) seems to behave just fine.
Assignee | ||
Comment 7•21 years ago
|
||
Jerry, could you look in ftp://ftp.mozilla.org/pub/mozilla.org/mozilla/tinderbox-builds/ and see whether the builds there help? Those go back further than I expected...
Reporter | ||
Comment 8•21 years ago
|
||
Luckily, the 2003111209 build does work correctly as Christian reported.
Comment 9•21 years ago
|
||
I downloaded the latest build and I do see this bug now (2003111413)... so regression range seems to be 11/12/2003 09:00 to 11/13/2003 14:00
Comment 10•21 years ago
|
||
maybe this one? 11/12/2003 20:13 bzbarsky%mit.edu mozilla/ layout/ xul/ base/ src/ nsBoxFrame.cpp 1.224 1/5 BoxFrame should only be the point target in the background layer, and should propagate background layer point requests to kids. Bug 224013, r+sr=dbaron (bonsai link for aforementioned range is http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=11%2F12%2F2003+09%3A00&maxdate=11%2F13%2F2003+14%3A00&cvsroot=%2Fcvsroot )
Assignee | ||
Comment 11•21 years ago
|
||
Yeah, that's the one I was thinking about. Could someone clearly explain to me what this bug is about? Menus work fine over here in a Linux build, so what are the _exact_ steps to reproduce?
Reporter | ||
Comment 12•21 years ago
|
||
For one example: 1. Click on the Tools menu. 2. Run your mouse down the right side of the menu (so it's near the right margin). WHAT HAPPENS: Nothing SHOULD HAPPEN: Each menu item should activate as you slide down the menu.
Reporter | ||
Comment 13•21 years ago
|
||
Make sure you're in-between the arrows and the words. It seems contrived, but believe me it gets really weird real fast when using Mozilla. I noticed it within a few minutes.
Assignee | ||
Comment 14•21 years ago
|
||
Ah, there we go. That should have been in comment 0... I do see this on Linux, and this is indeed a regression from bug 224013. Anyone know how menu "activation" works exactly? ;)
Assignee: hyatt → bz-vacation
OS: Windows XP → All
Hardware: PC → All
Summary: Menu items don't activate properly → Menu items don't activate properly (see comment 12)
Comment 15•21 years ago
|
||
fwiw, another way this bug shows is this: click a bit to the right/left of the menu text in the menu bar, and observe that no menu appears
Assignee | ||
Comment 16•21 years ago
|
||
Assignee | ||
Updated•21 years ago
|
Keywords: helpwanted
Assignee | ||
Comment 17•21 years ago
|
||
Ok, this time I decided I'd look at all the GetFrameForPoint impls in XUL, since they seem to have some similar issues... The ones I didn't change seem fine as they are.
Assignee | ||
Updated•21 years ago
|
Attachment #135510 -
Attachment is obsolete: true
Assignee | ||
Comment 18•21 years ago
|
||
Comment on attachment 135517 [details] [diff] [review] Slightly more extensive David, what do you think? I've run through a bunch of our UI trying to exercise various XUL issues and not seeing any problems (and the menus are fixed).
Attachment #135517 -
Flags: superreview?(dbaron)
Attachment #135517 -
Flags: review?(dbaron)
Comment on attachment 135517 [details] [diff] [review] Slightly more extensive >Index: nsDeckFrame.cpp >+ // if its not in our child just return us. >+ *aFrame = this; Need to check background paint layer before doing this (and do the correct return value). >Index: nsMenuFrame.cpp > if ((result != NS_OK) || (*aFrame == this)) { Don't test |*aFrame| here -- it's a UMR, no? >Index: nsSliderFrame.cpp >+ >+ if (!mRect.Contains(aPoint)) >+ return NS_ERROR_FAILURE; > > // always return us (if visible) > if (GetStyleVisibility()->IsVisible()) { > *aFrame = this; > return NS_OK; > } > > return NS_ERROR_FAILURE; Just merge these, i.e., if (mRect.Contains(aPoint) && Get... >Index: nsSplitterFrame.cpp >+ nsresult rv = nsBoxFrame::GetFrameForPoint(aPresContext, aPoint, aWhichLayer, aFrame); >+ if (rv == NS_ERROR_FAILURE && mRect.Contains(aPoint)) { Probably you should also test |aWhichLayer == ...FOREGROUND| in between these two tests. >+ *aFrame = this; >+ rv = NS_OK; >+ } > >+ return rv; > } Otherwise, looks fine to me.
Attachment #135517 -
Flags: superreview?(dbaron)
Attachment #135517 -
Flags: superreview+
Attachment #135517 -
Flags: review?(dbaron)
Attachment #135517 -
Flags: review+
Assignee | ||
Comment 20•21 years ago
|
||
> Need to check background paint layer before doing this Done. > Don't test |*aFrame| here -- it's a UMR, no? No. *aFrame is looked at only if nsBoxFrame::GetFrameForPoint returned NS_OK. I'll change that to use NS_FAILED to make things clearer. > Probably you should also test |aWhichLayer == ...FOREGROUND| in between these > two tests. Done.
Assignee | ||
Comment 21•21 years ago
|
||
Assignee | ||
Comment 22•21 years ago
|
||
Checked in.
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Component: XP Toolkit/Widgets: Menus → XUL
QA Contact: shrir → xptoolkit.widgets
You need to log in
before you can comment on or make changes to this bug.
Description
•