Drop-down comboboxes appear below Dock

VERIFIED FIXED in mozilla0.9.5

Status

()

VERIFIED FIXED
18 years ago
18 years ago

People

(Reporter: the_great_spam_bin, Assigned: mikepinkerton)

Tracking

Trunk
mozilla0.9.5
PowerPC
macOS
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [PDT+][ETA 10.08] [OSX+])

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

18 years ago
Although as of recent builds, XUL popups can no longer appear below the Dock
(bug 83570), drop-down comboboxes still can. I think that the reason for this
problem is that in nsComboboxControlFrame::PositionDropdown(), the position of
the drop-down is calculated using the raw height of the screen instead of an
adjusted height that compensates for the Dock. This is a pretty annoying and
probably easy to fix UI issue.

Comment 1

18 years ago
No, not much is easy to fix, especially with the combobox. It does get the 
screen height. I don't think the APIs reflect anything else. Hyatt, was XUL 
changed to compensate for this?
(Reporter)

Comment 2

18 years ago
In nsMenuPopupFrame::SyncViewWithFrame(), the XUL menu is positioned according
to  the bounds given by nsIDOMScreen::GetAvailTop(), ...Left(), ...Width(), and
...Height(). Bug 83570 modified nsScreenMac::GetAvailRect() to use a native API
which returned the bounding rectangle of the screen minus the menu bar and the
dock. I'm not sure why the combobox isn't using the screen's GetAvail... methods
to position the drop-down.

Updated

18 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 3

18 years ago
They aren't using them, because no one ever bothered to tell me about them when 
they went in.
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9.6
this should be simple enough and i'm on a roll today. taking.
Assignee: rods → pinkerton
Status: ASSIGNED → NEW
Created attachment 52054 [details] [diff] [review]
make GetScreenHeight use client rect not full rect
(Assignee)

Updated

18 years ago
Status: NEW → ASSIGNED
rod, can you review the patch? bascially it changes
nsFormControlFrame::GetScreenHeight to use the client area intstead of the full
area of the screen. the client area subtracts off the menubar, dock, taskbar,
etc which i can't see you ever wanting to position any form element over.
(Reporter)

Comment 7

18 years ago
This is technically wrong for Windows because drop-downs are allowed to cover
the taskbar. XUL popups in nsComboboxControlFrame::PositionDropdown() checks
whether the OS allows popups to cover the OS bars and then decides whether to
use the client rect or the screen rect.
ok, you're right about win32 and covering the taskbar, but i disagree that
nsComboboxControlFrame::PositionDropdown takes this into account. There's
nothing in that routine that checks that flag in look&feel.
(Reporter)

Comment 9

18 years ago
opps i meant nsMenuPopupFrame::SyncViewWithFrame(). heh.
Created attachment 52057 [details] [diff] [review]
check lookandfeel in GetScreenHeight
(Assignee)

Updated

18 years ago
Attachment #52054 - Attachment is obsolete: true
latest patch checks lookandfeel like xul popups do. both mac and win32 should be
good to go now. patch also adds a contractid for lookandfeel.

Comment 12

18 years ago
Looks right to me. r/sr=sfraser

Comment 13

18 years ago
You do not need to go get a LookAndFeel via "create instance", the PresContext 
caches a reference for speed, do this instead:

    nsCOMPtr<nsILookAndFeel> lookAndFeel;
    aPresContext->GetLookAndFeel(getter_AddRefs(lookAndFeel));

(Also, check the tab settings the indentation looks wierd in the patch)

With the above change r=rods
Created attachment 52081 [details] [diff] [review]
detabs and gets look&feel from presContext.
final version of patch, no longer creates instance of look&feel and de-tabs that
routine once and for all. thanks for the pointer rod.
(Assignee)

Updated

18 years ago
Attachment #52057 - Attachment is obsolete: true

Comment 16

18 years ago
Comment on attachment 52081 [details] [diff] [review]
detabs and gets look&feel from presContext.

setting r and sr flags since sfraser and rods have given them
Attachment #52081 - Flags: superreview+
Attachment #52081 - Flags: review+
let it bake on the trunk for a couple of days, then let's see if we can take it.
Keywords: nsbranch+
Whiteboard: [PDT]
Comment on attachment 52081 [details] [diff] [review]
detabs and gets look&feel from presContext.

a=asa (on behalf of drivers) for checkin to 0.9.5.
Attachment #52081 - Flags: approval+
(Assignee)

Updated

18 years ago
Target Milestone: mozilla0.9.6 → mozilla0.9.5
landed on trunk, waiting for go-ahead on branch

Updated

18 years ago
Whiteboard: [PDT] → [PDT][ETA 10.08] [OSX+]
pls check this into the branch - PDT+, after they have been verified on the trunk
Whiteboard: [PDT][ETA 10.08] [OSX+] → [PDT+][ETA 10.08] [OSX+]
landed on MOZILLA_0_9_4_BRANCH.
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED

Comment 22

18 years ago
verified fixed on branch build :- 2001-10-11-04-0.9.4 macOS X
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.