Closed Bug 503791 Opened 15 years ago Closed 15 years ago

Drop down menu at page bottom edge not displayed completely

Categories

(Core :: Layout: Form Controls, defect)

Product:
Core
Component:
Layout: Form Controls
Platform:
x86
Windows XP
Type:
defect
Priority:
Not set
Severity:
minor

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla1.9.3a1
Tracking Flags:
Tracking Status
status1.9.2 --- beta1-fixed

People

(Reporter: fuzzykiller, Assigned: tnikkel)

References

(
URL
)

Details

Attachments

(2 files)

make sure that the device context has a widget
1.28 KB, patch
dbaron
: review+
dbaron
: approval1.9.2+
Details | Diff | Splinter Review
teach ComboboxControlFrame that the origin of the screen is not always (0,0)
7.13 KB, patch
dbaron
: review+
Details | Diff | Splinter Review 
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.1) Gecko/20090624 Firefox/3.5 (.NET CLR 3.5.30729)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.1) Gecko/20090624 Firefox/3.5 (.NET CLR 3.5.30729)

Part 1
When a drop down menu (select form tag with height 1) of sufficient height is at or near the bottom of the displayable area of a secondary screen, it is not displayed completely but goes beyond the edge of the displayable area.
When a drop down menu of sufficient width is at or near the right edge of the displayable area, it is not displayed completely but goes beyond the edge of the displayable area.

Reproducible: Always

Steps to Reproduce:
Part 1:
1. Open drop down menu near or at bottom of displayable area on a secondary screen with no taskbar.

Part 2:
1. Open drop down menu near or at right edge of displayable area.
Actual Results:  
Part 1:
Drop down menu opens downwards. Most of elements not accessible.

Part 2:
Drop down menu is cut of. Scrollbar is also cut of if width is sufficient.

Expected Results:  
Part 1:
Drop down menu opens upwards when it is longer than displayable area below.

Part 2:
Drop down menu is shifted to left so it is displayed completely.

I am using a dual screen setup with a rotated monitor to the left and a normal monitor to the right.
Left screen: 1024x1280
Right screen: 1440x900
Daniel, can you please provide a screenshot which shows all your mentioned items?
Version: unspecified → 3.5 Branch
Daniel, these screenshots are not really helpful. You should not cut-off that many parts of the Firefox window. It would be great to see at least the window borders.

Marcia, do we have a multi-screen setup in the QA lab?
Daniel, have you the same configuration as described on bug 422067?
Yes, my secondary display has negative coordinates. However, I do have scrollbars. Also, part 2 is not related to a dual screen configuration and occurs on Firefox 3.0.11 as well.
(In reply to comment #6)
> Yes, my secondary display has negative coordinates. However, I do have
> scrollbars. Also, part 2 is not related to a dual screen configuration and
> occurs on Firefox 3.0.11 as well.

Please don't submit two different issues in one bug. Each one should only cover a specific area.

Neil, do we have another bug which covers all those miss-placements of the drop down menus? I cannot find one.
The screenshot suggests that these are the menus from <select> boxes. If so, this is a form controls problem and not really something I know much about.
Roc, this is layout or? Do you know of an existing bug? If not I think we should combine it with bug 422067.
Component: Menus → Layout: Form Controls
Product: Firefox → Core
QA Contact: menus → layout.form-controls
Version: 3.5 Branch → 1.9.1 Branch
It's layout: form controls. Probably nsComboboxControlFrame::AbsolutelyPositionDropdown
Confirming based on screenshots.
Status: UNCONFIRMED → NEW
Ever confirmed: true
The left/right issue is bug 280014.

The up/down issue is odd, though.  I have no idea why that's happening in this case; we have explicit code to prevent that...
Thanks Boris. So let this bug be covering the up/down part.
Summary: Drop down menu at page bottom and/or right edge not displayed completely → Drop down menu at page bottom edge not displayed completely
And just to check...  The up/down thing is specific to the multi-monitor setup?
To me, yes, and I replicate it only on secondary monitor.
My secondary screen is placed below and left to the primary meaning that topmost pixel on secondary screen is "below" the topmost pixel of primary screen. Also, under this setup, the list gets cut only on top so I'm guessing the wrong value (primary screen instead of current screen) is being used for calculating whether there's enough space.
Hmm.  That's quite possible (and I think we have bugs covering that already).  We just get the rect or client rect using the functions here:  http://mxr.mozilla.org/mozilla-central/source/gfx/public/nsIDeviceContext.h#379

Do those not do the right thing on Windows?
Whiteboard: DUPEME
So there are a couple of problems here.

If the device context does not have a widget then we fall back to the primary screen when determining the screen rect in this code (see nsThebesDeviceContext::FindScreen). This was regressed by http://hg.mozilla.org/mozilla-central/rev/253c445cc416 (bug 479749) and then fixed by bug 482928. But then regressed again less severely by http://hg.mozilla.org/mozilla-central/rev/9b8e2261d5fa (bug 352093, compositor).

The other problem is that ComboboxControlFrame does its drop down/up calculation assuming the origin of the screen is (0,0) (or at least that the top is at 0).

I found a whole bunch of similar looking bugs without even trying. I'm just going to post patches to fix these problems and once they are fixed I'll look at those other bugs and see what their status is.
Assignee: nobody → tnikkel
OS: Windows 7 → Windows XP
Hardware: x86_64 → x86
Whiteboard: DUPEME
Version: 1.9.1 Branch → Trunk
Feel free to reassign review if this is not a good choice.
Attachment #394242 - Flags: review?(roc)
I changed nsFormControlFrame::GetScreenHeight to return the screen rect in app units (both users wanted app units anyways) and used the rect instead of just the screen height.

Robert, if you're not a good reviewer for this I'm sure you know who is.
Attachment #394243 - Flags: review?(roc)
Comment on attachment 394243 [details] [diff] [review]
teach ComboboxControlFrame that the origin of the screen is not always (0,0)

Handing off to dbaron since I'm going out of town
Attachment #394243 - Flags: review?(roc) → review?(dbaron)
Comment on attachment 394242 [details] [diff] [review]
make sure that the device context has a widget

Handing off to dbaron since I'm going out of town
Attachment #394242 - Flags: review?(roc) → review?(dbaron)
Comment on attachment 394242 [details] [diff] [review]
make sure that the device context has a widget

I wonder if this fixes the issue I had with media queries and multi-monitor setups.  (See bug 507457 comment 13.)

r=dbaron
Attachment #394242 - Flags: review?(dbaron) → review+
Comment on attachment 394243 [details] [diff] [review]
teach ComboboxControlFrame that the origin of the screen is not always (0,0)

r=dbaron
Attachment #394243 - Flags: review?(dbaron) → review+
I'm putting this in my tree for try-server shortly and then (presumably) checkin later.
http://hg.mozilla.org/mozilla-central/rev/8130715aa7aa
http://hg.mozilla.org/mozilla-central/rev/4e8eb6276c48


Note that if you want this on 1.9.2, you'll need to request approval for that.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3
Keywords: checkin-needed
We should probably take at least the widget patch, since that part is a regression on the 1.9.2 branch.
Flags: wanted1.9.2?
Attachment #394242 - Flags: approval1.9.2?
Target Milestone: mozilla1.9.3 → mozilla1.9.3a1
Daniel, can you please check the build below if it fixes your problem? Please run it with a fresh profile because it's a pre-alpha release. See http://support.mozilla.com/en-US/kb/Managing+profiles

http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2009/08/2009-08-18-05-mozilla-central/
Comment on attachment 394242 [details] [diff] [review]
make sure that the device context has a widget

a1.9.2=dbaron
Attachment #394242 - Flags: approval1.9.2? → approval1.9.2+
Keywords: checkin-needed
Whiteboard: [needs 192 landing]
Yes, the Minefield build fixes the problem. The combobox menu now opens upwards on my secondary screen.
Thanks Daniel for your feedback. Lets mark this bug as verified then.

Timothy, can we land the patch on 1.9.2?
Status: RESOLVED → VERIFIED
The one patch that has approval can be landed by anyone (I asked only for approval on that one because it was a regression on the 1.9.2 branch). I don't know if Daniel's specific setup needs both patches or not, but I'm fine with approval being asked on the other patch.
Pushed that patch as http://hg.mozilla.org/releases/mozilla-1.9.2/rev/69053df2a4ed

Can someone who can reproduce the bug try tomorrow's 1.9.2 nightly to see whether that fixes it?
Keywords: checkin-needed
Whiteboard: [needs 192 landing]
From testing, both patches fix real, but different, problems. The second patch fixes an issue that is very old. Based on that we can choose to take the second patch on 1.9.2 based on what is acceptable to take on 1.9.2 at this point.
Oh, does it mean we shouldn't call it beta1-fixed until the second patch has been landed?
Whatever will cause less confusion.
(In reply to comment #34)
> Daniel, it would be great if you could also check the following build:

That doesn't fix it.
Given Daniel's last comment we should remove the flag.
status1.9.2: beta1-fixed → ---
I'm confused. It seems that some of the issues addressed in this bug were recent regressions but others are not. Is it true that the regression issues are now all fixed on 1.9.2?
The 'make sure that the device context has a widget' patch fixes a regression that happened on 1.9.2, and it has landed on 1.9.2. The other patch fixes a bug that is quite old, old enough that I don't know when it regressed; it has not landed on 1.9.2. So this bug is done from a 1.9.2 perspective unless someone really wants the other patch on 1.9.2.
But it doesn't fix the initial problem described in this bug. It would have been better to split it into another bug. But how should we handle it now. We cannot tell it's fixed on 1.9.2 because it's missing the additional fix.
It's probably simplest to just file a new bug for the original issue.
Flags: wanted1.9.2? → wanted1.9.2+
Getting this off the approved-but-not-yet-landed list.
status1.9.2: --- → beta1-fixed
I'm afraid the error is still present in this build.
Sorry that was my fault. The needed patch hasn't pushed to 1.9.2.

Timothy, what are the risks on your ' teach ComboboxControlFrame that the origin of the screen is not always (0,0)' patch to get landed on 1.9.2? Could we ask for approval? If we need a new bug for the 1.9.2 issue only we should do it now otherwise it will be too late. Sorry, that I have missed that.
It's pretty late in the game for 1.9.2, and I don't think there is much upside to landing it on 1.9.2. So if you need to file a bug, go ahead.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: