Closed
Bug 1267636
Opened 8 years ago
Closed 8 years ago
DevEdition theme and others cropped in menubar when switching to lowdpi monitor
Categories
(Core :: Widget: Win32, defect)
Tracking
()
VERIFIED
FIXED
mozilla49
Tracking | Status | |
---|---|---|
firefox46 | --- | unaffected |
firefox47 | --- | verified |
firefox48 | --- | verified |
firefox49 | --- | verified |
People
(Reporter: bmaris, Assigned: jfkthame)
References
Details
(Keywords: regression)
Attachments
(2 files)
11.54 KB,
image/png
|
Details | |
1.76 KB,
patch
|
emk
:
review+
Sylvestre
:
approval-mozilla-aurora+
ritu
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
[Affected versions]: - Firefox 47 beta 1 - Latest Developer Edition 48.0a2 - Latest Nightly 49.0a1 [Affected platforms]: - Windows 10 64-bit [Steps to reproduce]: Prerequisits: hidpi and lowdpi monitors. 1. Start Firefox 2. Install DevEdition theme for Nightly and Beta. (https://addons.mozilla.org/en-US/firefox/addon/devedition-theme-enabler/) 3. Move Firefox from hidpi to lowdpi monitor. [Expected result]: - Theme is not altered in menubar. [Actual result]: - Theme is altered in menubar. If theme is not installed, an increase of menubar can be seen when firefox is moved to lowdpi screen. [Regression range]: - This is a recent regression: [2016-03-11, 2016-03-12] Last good revision: 102886e9ac63b3fa33a6f1b394aea054abce2dfd (2016-03-11) First bad revision: 946ed22cad04431c75ab5093989dfedf1bae5a3e (2016-03-12) Pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=102886e9ac63b3fa33a6f1b394aea054abce2dfd&tochange=946ed22cad04431c75ab5093989dfedf1bae5a3e Maybe caused by: 68cabbc58a91 Jonathan Kew — Bug 1249496 - Don't apply dpi-based scaling for window titlebar dimensions when on a secondary display, because windows doesn't scale it. r=emk [Additional notes]: - This happens only when moving FF from hidpi to lowdpi (hidpi being Main display). - This also happens with other complete themes, eg: Lava Fox v2 - Windows 8.1 is not affected due to different windows theme, although the menubar is also increased on 8.1.
Comment 1•8 years ago
|
||
I thought this should have been addressed by bug 1256731... Jonathan, any idea what's going on here?
Assignee | ||
Comment 2•8 years ago
|
||
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
Assignee | ||
Comment 3•8 years ago
|
||
Handling the various theme metrics gets messy because while Windows doesn't (currently) scale the non-client area and controls according to per-monitor DPI, on Win10 we don't actually use the native Windows rendering of the controls. So while I don't really understand all the interactions going on here, the result of my experimentation across Win8.1 and Win10 with mixed DPI displays is that I think we should revert the GetMinimumWidgetSize part of bug 1249496, but only when running on Win10. This avoids the excess titlebar height that results in ugly DevEdition theme painting. I've started a try build at https://treeherder.mozilla.org/#/jobs?repo=try&revision=33ddf38e6930. Bogdan, could you please test with this build when it's ready, and confirm whether it fixes the problem you were seeing here? Thanks.
Flags: needinfo?(jfkthame) → needinfo?(bogdan.maris)
Assignee | ||
Updated•8 years ago
|
Attachment #8753376 -
Flags: review?(VYV03354)
Comment 4•8 years ago
|
||
Comment on attachment 8753376 [details] [diff] [review] Adjust window titlebar metrics scaling on Win10 to improve the appearance of DevEdition and similar themes on a secondary lo-dpi display Excellent.
Attachment #8753376 -
Flags: review?(VYV03354) → review+
Assignee | ||
Comment 5•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/cb4b7abddb9e13bf34c6b6a1865c449cbdaf7ec8 Bug 1267636 - Adjust window titlebar metrics scaling on Win10 to improve the appearance of DevEdition and similar themes on a secondary lo-dpi display. r=emk
Reporter | ||
Comment 6•8 years ago
|
||
(In reply to Jonathan Kew (:jfkthame) from comment #3) > I've started a try build at > https://treeherder.mozilla.org/#/jobs?repo=try&revision=33ddf38e6930. > Bogdan, could you please test with this build when it's ready, and confirm > whether it fixes the problem you were seeing here? Thanks. I used this build and saw right from the start that when Firefox starts with a fresh profile the titlebar has some excess height but when moving the window, it automatically shrinks back to normal. This behavior can be seen on both lowdpi or hidpi.
Flags: needinfo?(bogdan.maris) → needinfo?(jfkthame)
Reporter | ||
Comment 7•8 years ago
|
||
Forgot to mention that after I move the window, I can't see any other issues related to this.
Comment 8•8 years ago
|
||
(In reply to Bogdan Maris, QA [:bogdan_maris] from comment #6) > (In reply to Jonathan Kew (:jfkthame) from comment #3) > > I've started a try build at > > https://treeherder.mozilla.org/#/jobs?repo=try&revision=33ddf38e6930. > > Bogdan, could you please test with this build when it's ready, and confirm > > whether it fixes the problem you were seeing here? Thanks. > > I used this build and saw right from the start that when Firefox starts with > a fresh profile the titlebar has some excess height but when moving the > window, it automatically shrinks back to normal. This behavior can be seen > on both lowdpi or hidpi. (In reply to Bogdan Maris, QA [:bogdan_maris] from comment #7) > Forgot to mention that after I move the window, I can't see any other issues > related to this. I suspect this is bug 1273094.
Assignee | ||
Comment 9•8 years ago
|
||
Thanks Bogdan. It sounds like the issue you describe is probably bug 1273094, as Gijs says. To help us double-check, I've pushed a try build of mozilla-beta plus the patch here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0c0a17ef7353 When that's ready, we should be able to confirm that it resolves this problem and does _not_ have the other (bug 1273094) issue.
Flags: needinfo?(jfkthame) → needinfo?(bogdan.maris)
Comment 10•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/cb4b7abddb9e
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Assignee | ||
Comment 11•8 years ago
|
||
Comment on attachment 8753376 [details] [diff] [review] Adjust window titlebar metrics scaling on Win10 to improve the appearance of DevEdition and similar themes on a secondary lo-dpi display Approval Request Comment [Feature/regressing bug #]: 1249496 [User impact if declined]: ugly titlebar rendering for users with Win10, complete themes such as Dev Edition, mixed display resolutions, and Firefox running on a low-dpi secondary monitor [Describe test coverage new/current, TreeHerder]: tested manually [Risks and why]: minimal risk, as the patch is extremely localized in its effect (dpi scaling of titlebar metrics) and is restricted to Win10 only [String/UUID change made/needed]: none
Attachment #8753376 -
Flags: approval-mozilla-beta?
Attachment #8753376 -
Flags: approval-mozilla-aurora?
Comment 12•8 years ago
|
||
Bogdan, could you verify this before we uplift it into beta? I will take it in aurora so that we improve the coverage
Comment 13•8 years ago
|
||
Comment on attachment 8753376 [details] [diff] [review] Adjust window titlebar metrics scaling on Win10 to improve the appearance of DevEdition and similar themes on a secondary lo-dpi display Improve dev edition l&f, taking it.
Attachment #8753376 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Assignee | ||
Comment 14•8 years ago
|
||
(In reply to Sylvestre Ledru [:sylvestre] from comment #12) > Bogdan, could you verify this before we uplift it into beta? Note: see comment 9 for the tryserver build of beta+patch, for verification.
Assignee | ||
Comment 15•8 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/ca41d290daa0
Reporter | ||
Comment 16•8 years ago
|
||
Just checked using trybuild from comment 9 and I can't see the issue described in bug 1273094 anymore, it looks good.
Flags: needinfo?(bogdan.maris)
Comment on attachment 8753376 [details] [diff] [review] Adjust window titlebar metrics scaling on Win10 to improve the appearance of DevEdition and similar themes on a secondary lo-dpi display Fix was verified on a try build, Beta47+
Attachment #8753376 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Updated•8 years ago
|
Flags: qe-verify+
Reporter | ||
Comment 19•8 years ago
|
||
Verified this is fixed on Windows 10 64-bit using Firefox 47 beta 8, latest Developer Edition 48.0a2, latest Nightly 49.0a1.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in
before you can comment on or make changes to this bug.
Description
•