Closed Bug 1190769 Opened 4 years ago Closed 4 years ago

about:newtab should fall back to Tahoma as a font on Windows XP

Categories

(Firefox :: New Tab Page, defect)

29 Branch
Unspecified
Windows XP
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 47
Tracking Status
firefox47 --- fixed

People

(Reporter: phlsa, Assigned: phlsa)

References

(Blocks 1 open bug)

Details

(Whiteboard: [qx:spec][winxp])

Attachments

(2 files, 2 obsolete files)

The tiles on about:newtab try to use Segoe UI as the font even on Windows XP (where this font is not available). As a result, the system falls back to a really crappy version of Times.

We should add Tahoma as a fallback there (default UI font on XP).
Note that the gear menu on that same page already uses the correct fallback font.
Added Tahoma as a fallback font in the Windows-specific CSS file.
Assignee: nobody → philipp
Status: NEW → ASSIGNED
Attachment #8704155 - Flags: review?(dao)
Comment on attachment 8704155 [details] [diff] [review]
add Tahoma as fallback font for newtab tiles

This works, I guess, but it's still not quite the right way to approach fonts here.

Can you please try using something like this instead:

.newtab-title {
  font: message-box;
  font-size: 1em;
]
Attachment #8704155 - Flags: review?(dao)
Alright! I needed to re-adjust line-height as well to restore the old appearance.
I used px values since everything in the newtab CSS uses px.
Attachment #8704155 - Attachment is obsolete: true
Attachment #8705636 - Flags: review?(dao)
Comment on attachment 8705636 [details] [diff] [review]
change font handling in .newtab-title

Switching reviewers since Dão has limited availability at the moment.
Attachment #8705636 - Flags: review?(dao) → review?(gijskruitbosch+bugs)
Comment on attachment 8705636 [details] [diff] [review]
change font handling in .newtab-title

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

This patch seems to be built on the other patch and so it doesn't apply. If you're using mq, use qfold to fold the two patches. If you're using evolve / "real commits", use something like:

hg histedit -r <previous-rev>

and then use "fold" or "roll" to combine the two commits.

::: browser/themes/windows/newtab/newTab.css
@@ +15,5 @@
>  
>  .newtab-title {
> +  font: message-box;
> +  font-size: 13px;
> +  line-height: 30px;

Both of these properties are already set in newTab.css:

https://dxr.mozilla.org/mozilla-central/rev/584870f1cbc5d060a57e147ce249f736956e2b62/browser/base/content/newtab/newTab.css#182-183

so please don't set them again.
Attachment #8705636 - Flags: review?(gijskruitbosch+bugs) → review-
(In reply to :Gijs Kruitbosch from comment #5)
> ::: browser/themes/windows/newtab/newTab.css
> @@ +15,5 @@
> >  
> >  .newtab-title {
> > +  font: message-box;
> > +  font-size: 13px;
> > +  line-height: 30px;
> 
> Both of these properties are already set in newTab.css:
> 
> https://dxr.mozilla.org/mozilla-central/rev/
> 584870f1cbc5d060a57e147ce249f736956e2b62/browser/base/content/newtab/newTab.
> css#182-183
> 
> so please don't set them again.

Oh, I guess because you're using font, this gets overridden, and message-box isn't a valid value for font-family. I agree with Dão that we shouldn't be using px to size the font and line-height. Considering it's already being done, I guess we can do it this way, but please file a followup bug to fix the new tab CSS to use em instead.
OK, that one should work now. I'll file the follow-up bug now...
Attachment #8705636 - Attachment is obsolete: true
Attachment #8715833 - Flags: review?(gijskruitbosch+bugs)
Attachment #8715833 - Flags: review?(gijskruitbosch+bugs) → review+
https://hg.mozilla.org/mozilla-central/rev/3b7757168119
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 47
QA Whiteboard: [good first verify]
You need to log in before you can comment on or make changes to this bug.