All users were logged out of Bugzilla on October 13th, 2018

Firefox should not override browser.display.screen_resolution

RESOLVED FIXED

Status

()

--
major
RESOLVED FIXED
13 years ago
8 years ago

People

(Reporter: dbaron, Assigned: dbaron)

Tracking

({fixed1.8.1})

unspecified
x86
Linux
fixed1.8.1
Points:
---
Bug Flags:
blocking-firefox2 +
blocking1.8.0.2 -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [patch])

Attachments

(1 attachment)

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).
Flags: blocking1.9a1?
Flags: blocking1.8.0.2?
Flags: blocking-firefox2?
(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.)
Created attachment 209137 [details] [diff] [review]
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.
Assignee: nobody → dbaron
Status: NEW → ASSIGNED
Attachment #209137 - Flags: review?
Attachment #209137 - Flags: review? → review?(mconnor)
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.)
Whiteboard: [patch]

Updated

13 years ago
Attachment #209137 - Flags: review?(mconnor)
Attachment #209137 - Flags: review+
Attachment #209137 - Flags: branch-1.8.1+
Whiteboard: [patch] → [checkin needed (1.8 branch)][patch]
Checked in to trunk, although I still need to remove fontscaling.xul.
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED

Updated

13 years ago
Blocks: 325961

Comment 6

13 years ago
>  * 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?
I filed bug 326329 for the rest of the UI removal that needs to happen.
Flags: blocking1.8.0.2?
Flags: blocking1.8.0.2+
Flags: blocking-firefox2?
Flags: blocking-firefox2+
Checked in to MOZILLA_1_8_BRANCH.
Keywords: fixed1.8.1
Whiteboard: [checkin needed (1.8 branch)][patch] → [patch]
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.
Flags: blocking1.8.0.2+ → blocking1.8.0.2-
*** Bug 334113 has been marked as a duplicate of this bug. ***

Comment 11

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

Updated

12 years ago
Flags: blocking1.9a1?

Updated

8 years ago
You need to log in before you can comment on or make changes to this bug.