Closed
Bug 531289
Opened 15 years ago
Closed 15 years ago
Firefox doesn't obey system dpi settings anymore
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
status1.9.2 | --- | beta5-fixed |
People
(Reporter: pascalc, Assigned: mfinkle)
References
Details
(Keywords: regression)
Attachments
(3 files)
(deleted),
image/png
|
Details | |
(deleted),
patch
|
roc
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
dbaron
:
review+
|
Details | Diff | Splinter Review |
With today's nightlies, the UI fonts for Firefox trunk has become big.
See the attached screenshot to see the difference
Normal fonts with:
Mozilla/5.0 (X11; U; Linux i686; fr; rv:1.9.3a1pre) Gecko/20091125 Minefield/3.7a1pre
Bad font size with:
Mozilla/5.0 (X11; U; Linux i686; fr; rv:1.9.3a1pre) Gecko/20091126 Minefield/3.7a1pre
platform is Ubuntu Karmic, Gnome desktop.
Reporter | ||
Comment 1•15 years ago
|
||
Bug #530931 is fonts related for GTK and landed code change in the regression window, could be the cause of this bug?
Comment 2•15 years ago
|
||
Mark or Roc, could this be a regression from bug 530931?
Pascal, can you give us the changeset id's of both builds so we can check for all the checkins in this time frame?
Keywords: regression,
regressionwindow-wanted
Reporter | ||
Comment 3•15 years ago
|
||
Henrik, I woudln't know how to provide you this information but I am open to instructions for that :)
Comment 4•15 years ago
|
||
Just open about:buildconfig. You will see those links there.
Reporter | ||
Comment 5•15 years ago
|
||
20091125 build:
http://hg.mozilla.org/mozilla-central/rev/d76583175408
20091126 build:
http://hg.mozilla.org/mozilla-central/rev/77136b3d68fc
Comment 6•15 years ago
|
||
Thanks Pascal, here all the checkins in that time frame:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=d76583175408&tochange=77136b3d68fc
I was able to reproduce the bug when testing builds from 11/25 and 11/26. That's the time range when the patch on bug 530931 has been checked into 1.9.2.
Here steps how to reproduce this issue:
1. Open "System | Preferences | Fonts | Details" on Ubuntu
2. Change the DPI setting from 96dpi to e.g. 80dpi
3. Start latest Namoroka or Minefield build
With step 3 you can see that we don't obey the system dpi settings anymore. We always use a font size which applies to 96dpi. This bug will be highly visible on all laptops which mostly have another default dpi. As Pascal told me on IRC his laptop is using 90dpi per default.
Requesting blocking1.9.2.
Depends on: 530931
Flags: blocking-firefox3.6?
Keywords: regressionwindow-wanted
Summary: UI fonts is now big → Firefox doesn't obey system dpi settings anymore
Component: Menus → Graphics
Flags: blocking-firefox3.6?
Product: Firefox → Core
QA Contact: menus → thebes
Flags: blocking1.9.2+
Assignee: nobody → mark.finkle
Assignee | ||
Comment 7•15 years ago
|
||
The code is currently enforcing a minimum DPI:
http://mxr.mozilla.org/mozilla-central/source/gfx/thebes/src/gfxPlatformGtk.cpp#535
When I originally refactored the DPI code, this minimum-check only affected the Thebes drawing code, not the system/pango font code, as it does in m-192 and m-c:
http://mxr.mozilla.org/mozilla1.9.1/source/gfx/src/thebes/nsThebesDeviceContext.cpp#204
I'm not sure why GTK is the only platform that enforces the minimum, but I can add it back into the thebes code only. I could remove the minimum too, but I assume it's there for a reason.
Robert, what do you think?
Assignee | ||
Comment 8•15 years ago
|
||
I could also bring back the PlatformDPI, which would be 96dpi and used for the font code. That would leave Thebes to be the only consumer of the display DPI.
Assignee | ||
Comment 9•15 years ago
|
||
Simple fix. Uses a platform define in thebes code to enforce the minimum DPI only for GTK. This is very similar to the original DPI code, still found in 1.9.1
This is probably the safest thing to do for 1.9.2
Maybe we could file a followup for cleaning up trunk.
Attachment #414892 -
Flags: review?(roc)
Comment on attachment 414892 [details] [diff] [review]
patch
This is good for 1.9.2.
On trunk, can we just stop enforcing a minimum DPI on GTK2?
Attachment #414892 -
Flags: review?(roc) → review+
Assignee | ||
Comment 11•15 years ago
|
||
(In reply to comment #10)
> (From update of attachment 414892 [details] [diff] [review])
> This is good for 1.9.2.
OK, I'll and this on 1.9.2
> On trunk, can we just stop enforcing a minimum DPI on GTK2?
I think that's a good idea. If there is any fallout, we can adjust it.
Comment 12•15 years ago
|
||
I think we want to enforce the minimum 96dpi; Web pages become unusable at smaller DPIs because they specify font sizes in 'pt' that map to numbers of pixels that are too small to be legible at less than 96dpi.
We just don't want to apply that change to the GTK DPI that we use for interacting with GTK/GDK/pango.
Comment 13•15 years ago
|
||
And also see the comment just above the code you're modifying:
// A value of -1 means use the minimum of 96 and the system DPI.
// A value of 0 means use the system DPI. A positive value is used as the DPI.
// This sets the physical size of a device pixel and thus controls the
// interpretation of physical units such as "pt".
which is no longer what that code is doing.
Maybe we should just back out the whole series of patches given that the purpose of the most recent patch was to undo the change but leave the refactoring... maybe we just want to undo the whole thing?
Comment 14•15 years ago
|
||
(In reply to comment #13)
> Maybe we should just back out the whole series of patches given that the
> purpose of the most recent patch was to undo the change but leave the
> refactoring... maybe we just want to undo the whole thing?
Actually, never mind this. I think the refactoring is useful.
But I want to go through and review the whole set of patches and see what the overall diff looks like, because there have been a lot of patches and I'm pretty confused.
Assignee | ||
Comment 15•15 years ago
|
||
(In reply to comment #13)
> And also see the comment just above the code you're modifying:
>
> // A value of -1 means use the minimum of 96 and the system DPI.
> // A value of 0 means use the system DPI. A positive value is used as
> the DPI.
> // This sets the physical size of a device pixel and thus controls the
> // interpretation of physical units such as "pt".
>
> which is no longer what that code is doing.
Sorry I missed this comment before pushing. Yes, the patch makes -1 and 0 act like -1 on 192. The patch removes the minimum, so -1 and 0 work like 0 on trunk.
I can:
1. just update comments to be correct on 192 and trunk
2. add the full behavior back to 192
3. add full behavior back to trunk
4. 2 & 3
Thoughts?
Comment 16•15 years ago
|
||
I'd say either:
(1) land the full patch (attached and reviewed above) on both mozilla-central and 1.9.2
or
(2) restore the full behavior for both mozilla-central and 1.9.2, for at least GTK (also fixes bug 519108)
I'd slightly prefer (2). But I think dropping the 96dpi clamping is not acceptable; there's a good reason for it.
Comment 17•15 years ago
|
||
But (1)'s fine too; that just means landing the rest of what you've already landed on 1.9.2 on m-c as well.
Assignee | ||
Comment 18•15 years ago
|
||
This patch is for 192 and trunk. It puts back the full -1, 0, > 0 behavior for GTK, which appears to be the only code that used the -1 clamping mode. Other platforms only use the 0 (use system dpi) and the > 0 (override dpi) modes.
Attachment #415255 -
Flags: review?(dbaron)
Comment 19•15 years ago
|
||
Comment on attachment 415255 [details] [diff] [review]
revert to full behavior
>+#ifdef MOZ_ENABLE_GTK2
>+ if (prefDPI == -1) // Clamp the minimum dpi is 96dpi
I'd make this if (prefDPI < 0) for consistency with the old code.
r=dbaron with that, and thanks for fixing this
Attachment #415255 -
Flags: review?(dbaron) → review+
Assignee | ||
Comment 20•15 years ago
|
||
first patch
pushed m-c:
http://hg.mozilla.org/mozilla-central/rev/08321912dd62
pushed m-192:
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/2bdbd5e609bf
Assignee | ||
Comment 21•15 years ago
|
||
second patch
pushed m-c:
http://hg.mozilla.org/mozilla-central/rev/18633dc54642
pushed m-192:
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/eb059e3ff71d
Reporter | ||
Comment 22•15 years ago
|
||
I confirm that UI fonts behaviour is back to normal in Mozilla/5.0 (X11; U; Linux i686; fr; rv:1.9.3a1pre) Gecko/20091201 Minefield/3.7a1pre
Thanks
You need to log in
before you can comment on or make changes to this bug.
Description
•