Firefox doesn't obey system dpi settings anymore

RESOLVED FIXED

Status

()

Core
Graphics
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: pascalc, Assigned: mfinkle)

Tracking

({regression})

Trunk
x86
Linux
regression
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9.2 +

Firefox Tracking Flags

(status1.9.2 beta5-fixed)

Details

Attachments

(3 attachments)

(Reporter)

Description

8 years ago
Created attachment 414738 [details]
screenshot of font problem

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

8 years ago
Bug #530931 is fonts related for GTK and landed code change in the regression window, could be the cause of this bug?
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

8 years ago
Henrik, I woudln't know how to provide you this information but I am open to instructions for that :)
Just open about:buildconfig. You will see those links there.
(Reporter)

Comment 5

8 years ago
20091125 build:
http://hg.mozilla.org/mozilla-central/rev/d76583175408

20091126 build:
http://hg.mozilla.org/mozilla-central/rev/77136b3d68fc
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
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?
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.
Created attachment 414892 [details] [diff] [review]
patch

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+
(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.
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.
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?
(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.
(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?
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.
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.
Created attachment 415255 [details] [diff] [review]
revert to full behavior

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 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+
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
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
Status: NEW → RESOLVED
Last Resolved: 8 years ago
status1.9.2: --- → final-fixed
Resolution: --- → FIXED

Updated

8 years ago
Blocks: 519108
(Reporter)

Comment 22

8 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.