Closed
Bug 66848
Opened 24 years ago
Closed 23 years ago
Fix all the menu positioning problems that matter, and don't regress anything, beyotch.
Categories
(Core :: XUL, defect, P3)
Core
XUL
Tracking
()
RESOLVED
FIXED
mozilla0.9.3
People
(Reporter: bugs, Assigned: paulkchen)
References
Details
Attachments
(7 files)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
patch
|
Details | Diff | Splinter Review |
There are menu positioning problems with submenus of scrolling menus that are
themselves submenus.
This is due to a couple of problems:
- The widget that is being used to position menus is incorrect, it's not the
widget associated with the parent view of the spawning frame, but the root view
of the popup window widget that contains the frame.
- The root view is the client, not the screen, and better care when adding up
numbers needs to be made.
Patch coming.
Reporter | ||
Comment 1•24 years ago
|
||
Reporter | ||
Comment 2•24 years ago
|
||
Reporter | ||
Comment 3•24 years ago
|
||
Needed to add a 'GetWindowType' method to nsIWidget (& implement it on
nsBaseWidget, it is the analog of SetWindowType) so that I can figure out when a
view is the root view of a popup window. It seems that this sort of method might
be useful in other places too.
Status: NEW → ASSIGNED
Summary: Fix all the menu positioning problems that matter, and don't regress anything, beyotch. → Fix all the menu positioning problems that matter, and don't regress anything, beyotch.
+ PRBool ParentIsScrollableView(nsIView* aStartView);
+ PRBool ParentIsScrollableView(nsIView* aStartView)
?? and why aren't you using -u like everyone else?
Reporter | ||
Comment 5•24 years ago
|
||
How about I use whatever style of diff I like, and you mind your own business?
The lines you reference represent an internal helper method.
Comment 6•24 years ago
|
||
Use diff -u, beyotch!
/be
Reporter | ||
Comment 7•24 years ago
|
||
No, I prefer diff -c because I find it easier to read. As the layout/xul part of
this patch cannot easily be applied (since it contains portions of another patch
which has already been reviewed by pinkerton, and as a result are not posted
here), I chose readability over ease of patching. I could just as easily sent
out these changes via email, and this bug not existed at all. You pick.
Comment 8•24 years ago
|
||
Looks good r=rods
Comment 9•24 years ago
|
||
r=pinkerton, contingent on:
- building and tested on mac (i want to see it)
- removal/cleanup of |nsMacWindow::GetWindowType()|
Comment 10•24 years ago
|
||
i recind my r=pink. i tried this patch and context menus aren't correctly
positioned.
Reporter | ||
Comment 11•24 years ago
|
||
Reporter | ||
Comment 12•24 years ago
|
||
Comment 13•24 years ago
|
||
W2k, fresh source this morning, runs without patching.
I wanted to test out these patches. Patches still work on current tree.
Applying the patch to nsMenuPopupFrame (attach_id=27305) makes me crash on
gklayout.dll when opening any menu (also rightclick/contextmenu)
Applying both attach_id=27305 and attach_id=23670 (nsIWidget and nsBaseWidget)
makes me crash at startup in gkwidget.dll
Comment 14•24 years ago
|
||
Ignore my previous comment. After a complete clobber/rebuild it worked without
crashes.
The only positioning problem I see after applying this patch is that if a
submenu of a scrollable menu is ‘bounced’ of the bottom of the screen, or the
submenu is so large it is a scrollable submenu itself, it is positioned (one
menurow?) to low.
So a big scrollable submenu (larger then the height of the screen is positioned
(top=1, bottom=screenheight+1, height=screenheight) instead of (top=0,
bottom=screenheight).
Submenu’s aligned to the bottom of the screen are aligned to (screenheight+1)
instead screenheight. This seems independent of the scrolled position of the
scrollable menu.
Not sure my description is clear so I’ll attach two screenshots.
Comment 15•24 years ago
|
||
Comment 16•24 years ago
|
||
Reporter | ||
Comment 17•24 years ago
|
||
Throwing into .9.1 so I don't forget it. I'll investigate ajbanck's findings
during that cycle.
Target Milestone: --- → mozilla0.9.1
Comment 18•24 years ago
|
||
Assignee | ||
Comment 19•24 years ago
|
||
nav triage team:
marking nsbeta1+ and p3
Keywords: nsbeta1+
Priority: -- → P3
Comment 20•24 years ago
|
||
as discussed in team meeting, moving all Nav+ team members nsbeta1+ P3 bugs from
mozilla0.9.1 to mozilla0.9.2.
Target Milestone: mozilla0.9.1 → mozilla0.9.2
Assignee | ||
Comment 21•23 years ago
|
||
*** Bug 61662 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 22•23 years ago
|
||
nav triage team:
Reassigning to pchen who has a 0.9.1 bug that uses this patch. Will mark this
fixed once that bug is checked in.
Assignee: ben → pchen
Status: ASSIGNED → NEW
Target Milestone: mozilla0.9.2 → mozilla0.9.3
Assignee | ||
Comment 23•23 years ago
|
||
Oooops, I checked this fix in for mozilla0.9.1.
Status: NEW → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Component: XP Toolkit/Widgets: Menus → XUL
QA Contact: jrgmorrison → xptoolkit.widgets
You need to log in
before you can comment on or make changes to this bug.
Description
•