277278 - GUI for "always use my fonts" (browser.display.use_document_fonts)

Status: VERIFIED FIXED

(Camino Graveyard :: Preferences, enhancement)

Description

[Mitchell Lewandowski]

now that "open in tabs" has been implemented (thanks pink!), the only thing that
keeps firefox on my dock is it's ability to override the font sizes on many
pages (like gmail).  would it be possible to have camino have an "always use my
fonts" option, ala firefox?

Comment 1

[Mitchell Lewandowski]

would this just be a frontend to browser.display.use_document_fonts?  is this
preference (via about:config, and through prefs on ff) accessible to camino?

Comment 2

[Jasper]

Yes that would be it.

browser.display.use_document_fonts -> 0 or 1

Defines whether the browser uses the fonts specified in the web page for
display. Set to 1 to allow web pages to use fonts specified in page.

Do you still want a front end option for this?

Comment 3

[Mitchell Lewandowski]

(In reply to comment #2)
> Yes that would be it.
> 
> browser.display.use_document_fonts -> 0 or 1
> 
> Defines whether the browser uses the fonts specified in the web page for
> display. Set to 1 to allow web pages to use fonts specified in page.
> 
> Do you still want a front end option for this?

yes please, as the about:config in the nightly builds appears to be broken (at
least with the 03/06/2005 build) - it displays nothing.  i also think that it
would be good to mirror the checkbox mentality that ff has for this pref.  i
know that for me, it helps me set the fonts a little larger for reading.  is
this a trivial add?  i wouldn't mind setting the config option myself, however,
i am sure that the checkbox could benefit others...

Comment 4

[Chris Lawson (gone)]

Now that about:config works fine, do we still want this? I say we should probably do it along with the font prefs reorganisation outlined in bug 184102 and bug 175651.

We really ought to do all three at once, though.

cl

Comment 5

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

I'm not sure we do, but someone could convince me otherwise.

This pref is documented on cb.o: http://www.caminobrowser.org/support/hiddenprefs/#UseMyFonts

Comment 6

[Stuart Morgan]

There's GUI for "Always use my page and link colors", why not fonts as well?  Both are about page useability.

Comment 7

[philippe (part-time)]

* about:config is bad UI :-)
http://groups.google.com/group/mozilla.dev.apps.firefox/browse_thread/thread/457b9017eeddee26/2b126b0b4bac15c9#2b126b0b4bac15c9

* what Stuart says (comment #6).
Seeing the proposals in bug 184102, a small checkbox at the bottom of the font-panel would not make the UI anymore confusing or complex.

The option exist in IE Windoze, IE Mac, Firefox (and SeaMonkey), and Netscape 4 (iirc).

People who need this kind of options most are usually  not the most advanced users. Don't ask my *geeky* 77 years old neighbour to go to about:config, or worse, add a line to user.js/prefs.js.

PS -  Better use positive wording: 'Always use my fonts' is better than what Fx uses currently, I think.

Comment 8

[Chris Lawson (gone)]

To be clear, I'm in favour of a GUI option here, for the reasons Stuart and Philippe mention, and I'm especially in favour of doing it if we're going to be doing a major Appearance prefs overhaul.

cl

Comment 9

[froodian (Ian Leue)]

Attachment: Patch

Not sure if doing this was supposed to wait on other appearance prefpane bugs, but...

Comment 10

[froodian (Ian Leue)]

Attachment: Proposed nib change

Comment 11

[froodian (Ian Leue)]

Attachment: Screenshot of Proposed nib change

Comment 12

[Simon Fraser [no longer active]]

I wonder if the checkbox should go under Advanced...

Comment 13

[Chris Lawson (gone)]

(In reply to comment #12)
> I wonder if the checkbox should go under Advanced...
> 

Just to play devil's advocate, it isn't under Advanced in the other half of the Appearance pref pane. (Granted, there's no "Advanced" option at all for the Colors tab, but still...seems like that would make it undiscoverable.)

Ian, as far as I'm concerned, go ahead and fix this, and we'll fix the nib while we're touching it. Someone should file another bug for all our other nib changes if it hasn't been filed already. (We also need to agree on some sort of consistent spacing.)

cl

Comment 14

[Håkan Waara]

Comment on attachment 225221 [details] [diff] [review]
Patch

Can you just put some kind of comment somewhere indicating that a value of zero for that pref means we use only the user fonts?

That had me confused for a few seconds.  Looks good otherwise.

Comment 15

[froodian (Ian Leue)]

Attachment: Better documented Patch

Requesting addition r from cl

Comment 16

[Håkan Waara]

Comment on attachment 225270 [details] [diff] [review]
Better documented Patch

Assuming the UI change looks good. Code is ok.

Comment 17

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

I think it should be in Advanced (since it's a special-use, and potentially site-breaking, pref), but the current prefs in the Advanced sheet are all by-region, and this one is global.  Yet another reason that prefPane sucks.

Since reset is (currently) global, this pref makes more sense positioned between "Advanced" and "Reset" for the time being.

Also, IMO, the string should be more descriptive, something like "Always use my fonts instead of the ones specified by the page."

(I'm *still* trying to think of ways to fix that prefPane's UI....)

Comment 18

[froodian (Ian Leue)]

Attachment: Option 2 nib change

The real problem here (nibwise) is that the whole prefpane sucks (bug 184102).  I think the best solution for the time being is to put it below the advanced button like Smokey says as a temporary solution.  The problem with putting it in Advanced IMO is that a)Advanced is all region-specific (except for this), and b)It would hurt discoverability.

I think the name should remain the same, for parity with the "Use my colors" pref.  If it should be more descriptive, they can both be changed at the same time when 184102 gets fixed.

Comment 19

[froodian (Ian Leue)]

Attachment: Screenshot of Option 2 nib change

Comment 20

[Nick Kreeger]

Comment on attachment 225270 [details] [diff] [review]
Better documented Patch

Looks fine to me with the comments r+

Comment 21

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

While we're looking at this, should "advanced" in the hint text be "Advanced" instead, to match the button label?

Also, technically the "Always use my fonts" should be spaced above the "Reset" rather than sharing space with it; however, doing so will cause the prefPane to expand vertically and therefore require re-layout of the Colors tab, too :(  Don't know what we want to do here....

Comment 22

[froodian (Ian Leue)]

Attachment: Option 3 nib in Advanced Sheet

Hrm.  Maybe putting it in Advanced is the cleanest (least dirty?) solution.  Here's what that looks like.

And yeah, that hint text should be "Advanced."  If we decide on one of the main prefpane solutions, I'll whip up a new nib incorporating that.

Comment 23

[froodian (Ian Leue)]

Attachment: Screenshot of Option 3 nib

Comment 24

[Mike Pinkerton (not reading bugmail)]

+// The exposed pref has reverse logic from the internal pref,
+// so mUseMyFontsCheckbox = 1 maps to use_document_fonts = 0 

can you also make a comment about this when you set the pref when the panel loads? people might not see the later comment and stop and debug it. best to be clear. Or, better yet, provide a small wrapper function and all the setters can use that. Maybe that's overkill. Thoughts?

Comment 25

[froodian (Ian Leue)]

Attachment: Patch

This hides all the 0 == 1 logic away in useMyFontsPref: and setUseMyFontsPref:.  As you say Mike, it may be overkill (in which case I can just add more comments to what I had before), but I like it since it allows all the code elsewhere to pretend the pref has the same logic as what gets exposed in the UI.

Comment 26

[froodian (Ian Leue)]

Attachment: #camino developed nib

This nib keeps the checkbox out of the Advanced sheet, and attempts to follow the guidelines in http://wiki.caminobrowser.org/Development:Reviewing

Comment 27

[froodian (Ian Leue)]

Attachment: Screenshot of the colors/links side of latest nib

Comment 28

[froodian (Ian Leue)]

Attachment: Screenshot of the fonts side of latest nib

Comment 29

[Mike Pinkerton (not reading bugmail)]

+  [self clearPref:"browser.display.use_document_fonts"];

what's the purpose of this?

Comment 30

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 225750 [details] [diff] [review]
Patch

duh sr=pink

Comment 31

[Simon Fraser [no longer active]]

(In reply to comment #27)
> Created an attachment (id=225821) [edit]
> Screenshot of the colors/links side of latest nib

I don't like the spacing. The "Underline links" checkbox should be closer to the links color swatches than it is to the "Always..." checkbox, because it's link-related. Don't touch the current layout.

Comment 32

[froodian (Ian Leue)]

Attachment: Makes the change to Appearance.h too

Oops.  I made the changes in Appearance.h but forgot to diff it.  Thanks to smokey for trying to build with the patch and catching this. *embarrassed*

Comment 33

[froodian (Ian Leue)]

Attachment: This is really the last one

Just like the last one, but without f***ing up the whitespace.

Comment 34

[froodian (Ian Leue)]

Attachment: Screenshot of #camino developed nib, version 2

(In reply to comment #31)
> I don't like the spacing. The "Underline links" checkbox should be closer to
> the links color swatches than it is to the "Always..." checkbox, because it's
> link-related. Don't touch the current layout.

I agree that the current layout should be disturbed as little as possible, but if we're going to put the new pref in the main Fonts tab, we're gonna have to do a little bit of reflow on the colors & links tab to keep it OK looking.  How would you feel about a nib that had the same Fonts side as the last posted, and has a Colors & Fonts tab like this?

This keeps the same number of px between The "Underline links" checkbox and the links color swatches.  It just expands the distance between the two sets of color swatches, the two checkboxes, and adds some whitespace above the reset button.

Comment 35

[froodian (Ian Leue)]

Attachment: #camino developed nib, version 2

Comment 36

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Comment on attachment 225966 [details]
#camino developed nib, version 2

This looks nice and works OK for me (less bug 278820, of course).

Simon, is the Links & Colors side in attachment 225964 [details] OK with you?

Comment 37

[Simon Fraser [no longer active]]

Sure.

Comment 38

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 225966 [details]
#camino developed nib, version 2

sr=pink

Comment 39

[froodian (Ian Leue)]

fixed on trunk and 1.8 branch

Comment 40

[Paul Borokhov (lensovet)]

(In reply to comment #28)
> Created an attachment (id=225822) [edit]
> Screenshot of the fonts side of latest nib
> 
Hmm, I'm not really crazy about the "advanced" tip text that we have, in part because it uses the word "you", which I think is a little weird for a UI. No?
Here's my suggestion: « To set the minimum font size and other font types for this region, click Advanced... »
Also, this would go above the button, rather than below it.
What do you good people think?

Comment 41

[froodian (Ian Leue)]

This bug has been fixed.  File a new bug for any issues you have.

Comment 42

[[e-mail]]

Verified on 1.8 branch and trunk

Comment 43

[Samuel Sidler (old account; do not CC)]

Moving fixed "1.2" bugs to 1.1 where they were really fixed. Filter on CaminoFixed1.1 for bugmail purposes.