323962 - Firefox should not override browser.display.screen_resolution

Status: RESOLVED FIXED

(Firefox :: Settings UI, defect)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Firefox has overridden the browser.display.screen_resolution pref (Setting it to 96) since bug 274712 landed.  Unfortunately this shipped in Firefox 1.5.  This breaks the ability of point sizes to be useful for adapting to higher-resolution displays.

The preference itself only works on some platforms (bug 69205):  all of the relevant Unix-ish ports, plus OS/2 and BeOS (and really the UI for it should just be ifdef-ed out so it's only available on those platforms).  It might work on Mac (not sure if bug 11709 is still valid).

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(And, frankly, if there's some issue related to the UI for this pref, there's probably not much reason to have UI for this pref at all, although if we remove the UI we should probably change the name of the pref so that people who messed it up using the UI in an old version will have the damage undone.)

Comment 2

[:Gavin Sharp [email: [e-mail]]]

(see also bug 297141)

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch

This removes both the default value and the UI for the pref.

This removal of default value should not land on any branches that do not have bug 233082 (and preferably also bug 323964), both of which are pretty simple patches.

The case for removing the UI is:
 * it only works on some platforms
 * users really shouldn't need to mess with the pref **if it's correct** (why bug 233082 should also land)
 * the UI is currently buggy -- I watched vlad use the UI to unintentionally set it to "2" or "3" the other day by choosing the system setting option (which should set the magic value of "0")
 * the backend, especially for system fonts, may be somewhat buggy in handling dynamic changes of this pref (maybe that's what bug 308075 is)
 * users, even pretty advanced ones (see bug 297141, bug 283906), don't necessarily understand what this pref means.  It's only intended to change the interpretation of sizes specified in absolute units (pt, in, cm, mm, pc).

(FWIW, I intend to ask for Firefox-2 / Mozilla 1.8.1 approval for these patches.)

Notes:
 * I'm not confident I removed all the code related to the pref; worth reviewing carefully
 * This leaves a somewhat odd blank space in the UI, but I think it's the best simple alternative; changing the justification of the minimum font size pref would probably be a bad idea.

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Then again, the UI does have the fontscaling.xul bit, which actually is useful UI for this pref...  (I haven't tested whether it actually works, though, and it should provide better feedback about what the pref currently is.)

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Checked in to trunk, although I still need to remove fontscaling.xul.

Comment 6

[Steffen Wilberg]

>  * This leaves a somewhat odd blank space in the UI
We should consider removing this separator, since the other one with the groove is in the next line:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/browser/components/preferences/fonts.xul&rev=1.14&mark=257#257

And should this be removed from Thunderbird's fonts.xul as well?

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I filed bug 326329 for the rest of the UI removal that needs to happen.

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Checked in to MOZILLA_1_8_BRANCH.

Comment 9

[Daniel Veditz [:dveditz]]

It's really too late in 1.8.0.2 to take this kind of optional change, and it's not clear the 1.8.0 nomination was intended because approval was not sought on the patch nor was blocking requested on the bugs identified in comment 2 as prerequisites. If this needs to go on the 1.8.0 branch please re-nominate for 1.8.0.3 along with other required bugs.

Comment 10

[Phil Ringnalda (:philor)]

*** Bug 334113 has been marked as a duplicate of this bug. ***

Comment 11

[Felix Miata]

*** Bug 354505 has been marked as a duplicate of this bug. ***