102834 - Drop-down comboboxes appear below Dock

Status: VERIFIED FIXED

(Core :: Layout: Form Controls, defect)

Description

[Ari]

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

[rods (gone)]

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?

Comment 2

[Ari]

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.

Comment 3

[rods (gone)]

They aren't using them, because no one ever bothered to tell me about them when 
they went in.

Comment 4

[Mike Pinkerton (not reading bugmail)]

this should be simple enough and i'm on a roll today. taking.

Comment 5

[Mike Pinkerton (not reading bugmail)]

Attachment: make GetScreenHeight use client rect not full rect

Comment 6

[Mike Pinkerton (not reading bugmail)]

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.

Comment 7

[Ari]

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.

Comment 8

[Mike Pinkerton (not reading bugmail)]

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.

Comment 9

[Ari]

opps i meant nsMenuPopupFrame::SyncViewWithFrame(). heh.

Comment 10

[Mike Pinkerton (not reading bugmail)]

Attachment: check lookandfeel in GetScreenHeight

Comment 11

[Mike Pinkerton (not reading bugmail)]

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

[Simon Fraser [no longer active]]

Looks right to me. r/sr=sfraser

Comment 13

[rods (gone)]

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

Comment 14

[Mike Pinkerton (not reading bugmail)]

Attachment: detabs and gets look&feel from presContext.

Comment 15

[Mike Pinkerton (not reading bugmail)]

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.

Comment 16

[Steve Dagley]

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

Comment 17

[Jaime Rodriguez, Jr.]

let it bake on the trunk for a couple of days, then let's see if we can take it.

Comment 18

[Asa Dotzler [:asa]]

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.

Comment 19

[Mike Pinkerton (not reading bugmail)]

landed on trunk, waiting for go-ahead on branch

Comment 20

[Jaime Rodriguez, Jr.]

pls check this into the branch - PDT+, after they have been verified on the trunk

Comment 21

[Mike Pinkerton (not reading bugmail)]

landed on MOZILLA_0_9_4_BRANCH.

Comment 22

[Madhur Bhatia]

verified fixed on branch build :- 2001-10-11-04-0.9.4 macOS X