Submenus can go off the screen

RESOLVED FIXED

Status

()

Core
Widget: Cocoa
--
major
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: bz, Assigned: Josh Aas)

Tracking

({regression})

Trunk
x86
Mac OS X
regression
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

2.83 KB, patch
cbarrett
: review+
Stuart Parmenter
: superreview+
Details | Diff | Splinter Review
BUILD:  Current Mac trunk build

STEPS TO REPRODUCE:

1) Load https://bugzilla.mozilla.org/attachment.cgi?id=251778&action=edit
2) Move the window to the right-hand side of the screen
3) Ctrl-click in the attachment frame near the right edge
4) Hover the "This frame" option

EXPECTED RESULTS:  See a submenu.

ACTUAL RESULTS: Submenu is mostly off the screen, instead of going off to the left of the main menu.

This is a cocoa widget regression, at least based on the date when it appeared (2006-09-28) and the fact that it doesn't happen on Linux.
Flags: blocking1.9?
Keywords: regression
Flags: blocking1.9? → blocking1.9+
Assignee: joshmoz → cbarrett
Status: NEW → ASSIGNED
Created attachment 252675 [details] [diff] [review]
fix

Turns out nsCocoaWindow didn't implement WidgetToScreen, and the only place where that was asked of a window was in the popup menu code.
Attachment #252675 - Flags: review?(joshmoz)
(Assignee)

Comment 2

11 years ago
Comment on attachment 252675 [details] [diff] [review]
fix

Please null check mPopupContentView in both places. We don't want to crash if somebody tries to call that on a window that isn't a popup.

Also, we should probably implement the function for non-popups.
(Assignee)

Updated

11 years ago
Attachment #252675 - Flags: review?(joshmoz) → review-
Colin confirms that this fixes bug 340585 too (the ESM uses WidgetToScreen for mouse scroll events).
Blocks: 340585
Actually it doesn't seem to be working anymore. Just tried it and it's not working.

I think that bug is a focus (event?) issue -- if you'll notice the knob on the scrollbar is grey instead of blue -- that indicates the scrollbar is in the background.
(Assignee)

Comment 5

11 years ago
Created attachment 252749 [details] [diff] [review]
fix v2
Assignee: cbarrett → joshmoz
Attachment #252675 - Attachment is obsolete: true
Attachment #252749 - Flags: review?(cbarrett)
Comment on attachment 252749 [details] [diff] [review]
fix v2

tested, works. Is returning the same values my patch was.
Attachment #252749 - Flags: review?(cbarrett) → review+
(Assignee)

Updated

11 years ago
Attachment #252749 - Flags: superreview?(pavlov)

Updated

11 years ago
Attachment #252749 - Flags: superreview?(pavlov) → superreview+
(Assignee)

Comment 7

11 years ago
fixed on trunk
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.