Closed Bug 840881 Opened 8 years ago Closed 8 years ago

Wrong menu rendering with layout.css.devPixelsPerPx non-default (makes some menus unusable)

Categories

(Core :: Layout, defect)

21 Branch
x86_64
Linux
defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla23
Tracking Status
firefox21 + verified
firefox22 + verified
firefox23 + verified

People

(Reporter: kang, Assigned: jfkthame, NeedInfo)

References

Details

(Whiteboard: [verification pending bug 861270])

Attachments

(6 files)

This problem is GTK-builds-only.

When set to, for example, layout.css.devPixelsPerPx=1.1 in about:config, the problem occurs.
It's fine with the default value (-1.0).

As it's used for high DPI screens only in general, I suspect it's not a very common problem. It might become more common in the future as high DPI screens are becoming more mainstream.

This problem started to occur more or less a couple of month ago (unfortunately I don't remember when it started occurring, I though it was something on my end and I took long to check this). Current Firefox release is not yet affected.

See the attached picture for an example of the problem. This also occurs with menus opened with right-clicking in the page.
Component: Menus → Widget: Gtk
Product: Firefox → Core
Duplicate of this bug: 832490
Blocks: 712898
I'm also affected by this.

I've never worked on the Mozilla codebase before but if someone could give me some pointers as to where to begin (particularly with respect to this bug) I'd like to help resolve it.
Component: Widget: Gtk → Layout
Looks like confusion between CSS pixels and dev pixels (of course). Jonathan, can you reproduce this on Mac?
Hmm - I can't reproduce with the standard devPixelsPerPx setting (-1.0 = auto) on a Retina display, even though that involves CSS and dev pixels being different (1:2 ratio). But I can reproduce this with devPixelsPerPx set to a non-standard value.

Maybe a result of bug 814434 (though I haven't specifically confirmed that yet).
Yup, this broke in Nightly of 2012-12-08, which is when bug 814434 landed.
Blocks: 814434
Ubuntu 12.04 LTS got today Firefox 20. 

Firefox is unusable with layout.css.devPixelsPerPx set to custom value because all popups and menus are broken
I wonder if the fix for bug 819725 is what broke this?
This is essentially the same as the Windows widget patch in bug 819725 pt 2. With this patch, I'm seeing correct menu size/position on Linux, even with non-standard devPixelsPerPx values.
Assignee: nobody → jfkthame
Attachment #734380 - Flags: review?(roc)
Nominating for tracking as this is a pretty bad usability issue for Linux users with hi-dpi displays who want to use a custom css.layout.devPixelsPerPx setting to provide appropriate scaling.
A target of Firefox 23 seems like kindof a long wait seeing as this is a regression in Firefox 20. Firefox 19 worked fine so in my opinion this should be fixed in a point release.

Just my two cents. Thanks!
A target of Firefox 23 seems like kind of a long wait seeing as this is a regression in Firefox 20. Firefox 19 worked fine so in my opinion this should be fixed in a point release.

Just my two cents. Thanks!
https://hg.mozilla.org/mozilla-central/rev/a66e4f7da75f
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Comment on attachment 734380 [details] [diff] [review]
patch, apply default scale factor in gtk2 window move/resize methods

Carl, I agree we should fix this earlier than FF23; but that's just the current development version, which is where the fix has initially landed. (You could even test the latest Nightly build from http://nightly.mozilla.org/ to confirm that it solves the problem.)

Provided all goes well, I hope we can uplift this fix to FF22 and even FF21, so that it'll be available more quickly.


[Approval Request Comment]
Bug caused by (feature/regressing bug #): hidpi support

User impact if declined: unusable menus for Linux users who have set a custom devPixelsPerPx value

Testing completed (on m-c, etc.): tested locally, landed on m-c (also, directly equivalent to the code we already have on Windows to handle the same issue there)

Risk to taking this patch (and alternatives if risky): minimal risk - in gtk2-specific code, and has no impact except when the user has adjusted the layout.css.devPixelsPerPx setting (to compensate for high pixel density)

String or IDL/UUID changes made by this patch: none
Attachment #734380 - Flags: approval-mozilla-beta?
Attachment #734380 - Flags: approval-mozilla-aurora?
Thank you!
Comment on attachment 734380 [details] [diff] [review]
patch, apply default scale factor in gtk2 window move/resize methods

Low risk patch impacting Linux users who have hi-dpi screens and custom setting for layout.css.devPixelsPerPx .Impacts menu's in particular squishing them to a point that they are not visible as expected.

Approving on aurora,beta.
Attachment #734380 - Flags: approval-mozilla-beta?
Attachment #734380 - Flags: approval-mozilla-beta+
Attachment #734380 - Flags: approval-mozilla-aurora?
Attachment #734380 - Flags: approval-mozilla-aurora+
(In reply to Carl Thompson from comment #16)
> Thank you!

Hi Carl, this has now been uplifted.Can you please help us verify one of the Nightlies or the beta/aurora builds once the patch lands to confirm the issue is fixed for you ? Thanks !
Hi, I've given the change a try in the last nightly. It does fix things for most web pages but not all. Sites with embedded flash object (such as youtube when not in HTML5 mode) do not render correctly. The flash object is positioned in the wrong spot and when the page is scrolled it scrolls at a different speed than the rest of the page.

Also, while menus on web pages seem to work correctly (and web page context menus), menu's in Firefox's own chrome do not when layout.css.devPixelsPerPx is not 1 (or -1 which is apparently the same thing). For example, the the urlbar dropdown itself expands to the bottom of the screen and looks weird. Also, menus in the preferences dialog and in the addon's preferences dialogs quickly expand to fill the entire screen when selected. This makes these dialogs somewhat unusable without changing layout.css.devPixelsPerPx back to 1.

I'll attach screenshots to show these issues.
(In reply to Carl Thompson from comment #20)
> Hi, I've given the change a try in the last nightly. It does fix things for
> most web pages but not all. Sites with embedded flash object (such as
> youtube when not in HTML5 mode) do not render correctly. The flash object is
> positioned in the wrong spot and when the page is scrolled it scrolls at a
> different speed than the rest of the page.

Plugins would be a separate issue - if you could file a new bug report on that, it'd be great.

> Also, while menus on web pages seem to work correctly (and web page context
> menus), menu's in Firefox's own chrome do not when layout.css.devPixelsPerPx
> is not 1 (or -1 which is apparently the same thing).

Well, -1 (anything <= 0) means "automatic", which on Linux/Gtk2 currently translates to 1, as we don't have support for X11 or Gtk2 resolution scaling options - there are a couple of existing reports about that.

> For example, the the
> urlbar dropdown itself expands to the bottom of the screen and looks weird.

Yes, it seems to always take the full height, even if there are only a few items to be displayed. Though it still appears to work fine for me, at least.

> Also, menus in the preferences dialog and in the addon's preferences dialogs
> quickly expand to fill the entire screen when selected.

Huh, I see - when you initially open a menu in Preferences, it looks fine, but if you repeatedly open the same menu it grows wider each time. That's weird.
(In reply to Jonathan Kew (:jfkthame) from comment #24)

> ...

> > Also, menus in the preferences dialog and in the addon's preferences dialogs
> > quickly expand to fill the entire screen when selected.
> 
> Huh, I see - when you initially open a menu in Preferences, it looks fine,
> but if you repeatedly open the same menu it grows wider each time. That's
> weird.

Fo me, it quickly expands to fill the screen the _first_ time I click on the menu. No repeat opening required. Menus quickly take over the whole screen within a few seconds.
Ah, the results vary depending on the exact value you choose for devPixelsPerPx, at least in my testing. E.g. a value of 1.2 seems to work much better than 1.1.
nsIWidget describes the coordinates to Resize() etc as "expressed in the parent's coordinate system".  The child uses the same coordinate system as the parent, so I don't know why scaling should be applied.

Similarly, the nsIntRect passed to Create() is "specified in the parent's coordinate system" for child windows, and, as expected, no scaling is applied.  However, "for parentless widgets such as top-level windows" these are "in global CSS pixels", but no scaling is applied.

Could the Flash offset be caused by the patch here, or did that bug already exist?
Should the FIXED resolution be removed since there are still issues?
(In reply to Carl Thompson from comment #28)
> Should the FIXED resolution be removed since there are still issues?

Probably best to file a followup bug (or two: one for the Flash offset, another for the menu sizing). AFAICT, the patch here did fix the primary issue it aimed to fix, but is not sufficient to give stable behavior in all cases. In particular, the menu behavior seems to depend on the exact devPixelsPerPx value, with some scales working OK but others not.
Depends on: 861112
I've filed bug 861112 about the Flash positioning error. I think that's directly caused by the patch here needing to be more selective in what it targets.
Is there a another new bug for the remaining menu rendering issues? I personally think this bug should be reopened, particularly since the bug the original reporter shows in his screenshot still exists. I'll post a screenshot showing that the same bug is still here.
Hmm, I'm not seeing that behavior here any longer. What build are you running, and what specific setting of devPixelsPerPx?
Mozilla/5.0 (X11; Linux x86_64; rv:23.0) Gecko/20130409 Firefox/23.0 ID:20130409030855 CSet: b1fb34b07c17

layout.css.devPixelsPerPx;1.6
Depends on: 861270
Interesting, I don't see that with devPixelsPerPx=1.6 here (though I do get the "expanding popups" issue). But it looks like you have significantly larger UI fonts than I do, which may well affect how it behaves.

As a workaround, you might find that a different value (e.g. 1.5 or 1.7) works better for now, though given that you're getting different behavior from me, I can't be sure where the "sweet spot" (if any) will be for you.

Anyhow, I've filed bug 861270 to follow up on this; obviously, we'd like it to be stable and reliable for arbitrary values, not only certain specific settings.
OK, thanks, I'll try other values. I don't suppose the developers could revert whatever change broke this? It did used to work until v20 after all...
Yep, 1.62 works much better than 1.6 for me as far as the menus are concerned. It's strange that very small changes in the number make big visual differences; 1.6205 is noticeably different than 1.62.

My font size is set to 9 points if that's helpful.
Issues are still *not* fixed for me in latest nightly (Apr 15).
Since the original issue has not been fixed, should the status of this bug be changed from "RESOLVED FIXED" ? Thanks.
I can confirm this is not fixed in nightly. The drop-down menu for the URL is now being randomly resized.
The issue I think you're describing is filed as bug 861270. Please follow up there with any further details - exactly what settings are you using? Does the behavior vary if you try different devPixelsPerCSS values?
Keywords: verifyme
I do also have the issue described in bug 861270. However, I still have the issue described in this bug (as my last posted screenshot shows) as well which is why I believe this bug should be reopened.
Carl, one of the quirks of the way we manage bugs is that bug reports move from tracking symptoms to tracking the effects of patches.  As a patch has already landed here, it is best to use a new bug for the next patch, even if the remaining symptoms are the same.  Bugs are usually only reopened if the patch is backed out.  If bug 861270 doesn't cover what you are now experiencing, then best to clone this bug and edit as appropriate (see bottom right corner).
Given the common symptoms it's a bit confusing to me to determine if this is fixed or not. I can confirm the patch landed but until bug 861270 is resolved I don't think I can verify this is fixed. Karl, please advise if I'm incorrect.
Whiteboard: [verification pending bug 861270]
Yes - the patch here fixed the primary cause of the brokenness, but in many configurations the behavior would still be at least partially broken due to bug 861270. The patch for 861270 is currently on inbound, so once that is merged to central you should be able to verify the result of the two together.
(In reply to Anthony Hughes, Mozilla QA (:ashughes) from comment #44)
> Given the common symptoms it's a bit confusing to me to determine if this is
> fixed or not. I can confirm the patch landed but until bug 861270 is
> resolved I don't think I can verify this is fixed. Karl, please advise if
> I'm incorrect.

Yes, I think you are right that it's not possible to verify this as fixed, at least not without the changes in bug 861270.  That does open up the more general question of what process to follow when bugs fail verification.  I'd be happy with a way of marking them as such, and your whiteboard comment seems the best way we have to do that right now.
Can we call this verified if bug 861270 and bug 861112 are verified ?
I reproduce the issue in comment 0 on http://planet.mozilla.org/rss20.xml with layout.css.devPixelsPerPx=1.3 on nightly 2013-04-07, Ubuntu 12.04 x86. 
This looks fixed now in FF 21 RC.
Anything else that needs to be tested here ?
With bugs 861270 and 861112 also fixed, I think we're good - thanks.
Thanks. Calling this verified fixed based on Paul's testing here and the testing in the dependent bugs.
Keywords: verifyme
I have this bug when using two 4K monitors. Without layout.css.devPixelsPerPx set, everything's fine; with this setting changed to a non-default (1.5 in my case) the correct placement only appears when window is placed near the left edge of the first monitor; otherwise, the context and other menus pop to the left of the Firefox window. This includes context menus, address bar and search completion.
You should file a new bug for that, because this bug has been closed and verified.
Flags: needinfo?(fax.k.root)
You need to log in before you can comment on or make changes to this bug.