Closed Bug 1160456 Opened 5 years ago Closed 5 years ago

Add UI to configure the "fonts for mathematics" preferences

Categories

(Firefox :: Preferences, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
Firefox 41
Tracking Status
firefox41 --- fixed

People

(Reporter: fredw, Assigned: fredw)

References

Details

(Keywords: dev-doc-complete)

Attachments

(4 files, 1 obsolete file)

Bug 947654 will add font.*.x-math preferences options in order to configure the math fonts used for MathML. These should be configurable from the Firefox Fonts preference menu.
Attached patch Patch (obsolete) — Splinter Review
Comment on attachment 8603341 [details] [diff] [review]
Patch

Karl: can you try again the patch? It seems that it no longer works for me. Actually even when trying to set the fonts for "Cyrillic", the prefs are not updated. The only change I see is bug 1166161.
Attachment #8603341 - Flags: feedback+ → feedback?(karlt)
Comment on attachment 8603341 [details] [diff] [review]
Patch

(In reply to Frédéric Wang (:fredw) from comment #2)
> Karl: can you try again the patch? It seems that it no longer works for me.
> Actually even when trying to set the fonts for "Cyrillic", the prefs are not
> updated. The only change I see is bug 1166161.

The Cyrillic bug exists without this patch so it is not a problem with this patch, but a separate bug.
Attachment #8603341 - Flags: feedback?(karlt) → feedback+
Depends on: 1169567
No longer depends on: 1169567
Attachment #8603341 - Flags: review?(gijskruitbosch+bugs)
Attachment #8603341 - Flags: review?(gijskruitbosch+bugs)
Did you discuss this with a UX person? Mathematics isn't really a "language" in the same way that the other things in this list are. I'm not sure it makes sense to add the pref in this way, or that it would be discoverable for interested parties...
Flags: needinfo?(fred.wang)
(In reply to :Gijs Kruitbosch from comment #4)
> Did you discuss this with a UX person?

Sorry, I thought you were one, since you reviewed a patch for the font preference menu. I'll defer the review to someone else.

> Mathematics isn't really a "language"
> in the same way that the other things in this list are. I'm not sure it
> makes sense to add the pref in this way,

The point of reusing the existing UI instead of duplicating code and making it consistent with the other languages was actually to make it easier to be accepted... See bug 947654 comment 3.

> or that it would be discoverable for interested parties...

I'm not sure this is really a problem.
Flags: needinfo?(fred.wang)
Attachment #8603341 - Flags: review?(mak77)
Comment on attachment 8603341 [details] [diff] [review]
Patch

I am a browser peer like Marco. Both of us can review the patch. But we're not user experience people. You want someone like :phlsa and ask for ui-review.
Attachment #8603341 - Flags: review?(mak77) → ui-review?(philipp)
See Also: → 1170918
(In reply to Karl Tomlinson (ni?:karlt) from comment #3)
> > Karl: can you try again the patch? It seems that it no longer works for me.
> > Actually even when trying to set the fonts for "Cyrillic", the prefs are not
> > updated. The only change I see is bug 1166161.
> 
> The Cyrillic bug exists without this patch so it is not a problem with this
> patch, but a separate bug.

I can confirm that the patch works as expected again, after bug 1169567.
FYI, Florian added the new preference option to the release notes of Firefox 41 (see bug 947654 comment 81). I think it would be much better if this preference option have the associated UI so I hope this small patch could make it for Firefox 41.
Hey! Sorry, I can't do builds on my machine right now. Could you upload a screenshot of the changed UI for ui-review? Thanks!
Flags: needinfo?(fred.wang)
My builds at http://www.wg9s.com/mozilla/firefox/ include this patch.
Attached image math-pref-1.png
Flags: needinfo?(fred.wang)
Attached image math-pref-2.png
Attached image math-pref-3.png
I uploaded a couple of screenshots. So as you can see this is just the current font preference with a new item for "Mathematics". The main question is whether it's fine to just reuse the existing font preference for that purpose or whether mathematics deserve a separate UI. If the former is chosen, then another question is where to put this new item: in the (English) alphabetical order (so just after mayalayam) or at the end of the list, before the other special "x-unicode" language (as done in the patch)?
Flags: needinfo?(philipp)
Comment on attachment 8603341 [details] [diff] [review]
Patch

Review of attachment 8603341 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for the screen shots!
Putting it into that space makes sense. It would be preferable to put it into the normal alphabetical ordering though.
Attachment #8603341 - Flags: ui-review?(philipp) → ui-review+
Attached patch Patch V2Splinter Review
I'm reordering the language as suggested by Philipp Sackl. This actually makes it consistent with what I did for Thunderbird and SeaMonkey.
Attachment #8621217 - Flags: review?(gijskruitbosch+bugs)
Comment on attachment 8621217 [details] [diff] [review]
Patch V2

Review of attachment 8621217 [details] [diff] [review]:
-----------------------------------------------------------------

If UX is happy with it, this wfm.
Attachment #8621217 - Flags: review?(gijskruitbosch+bugs) → review+
Attachment #8603341 - Attachment is obsolete: true
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/d83d1212b239
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
Fred, is it desired behavior that the field for "Proportional" fonts, its "Size" field and the "Size" field for the monospace font are empty?
Flags: needinfo?(fred.wang)
(In reply to Aryx [:archaeopteryx][:aryx] from comment #22)
> Fred, is it desired behavior that the field for "Proportional" fonts, its
> "Size" field and the "Size" field for the monospace font are empty?

Yes, not all the font preferences were added for x-math in bug 947654. For example the font-size of the <math> is inherited from the parent (so that formulas have size consistent with the surrounding text) and font-family=monospace or font-family=sans-serif are rarely used for math formulas (or if they are, it's via the Unicode characters from https://en.wikipedia.org/wiki/Mathematical_Alphanumeric_Symbols). Perhaps some specific code should be added to ignore irrelevant fields for x-math
Flags: needinfo?(fred.wang)
You need to log in before you can comment on or make changes to this bug.