1164594 - New Tab shouldn't use text-transform: uppercase

Status: VERIFIED FIXED

(Firefox :: New Tab Page, defect)

Description

[Francesco Lodolo [:flod]]

Attachment: gear.png

Last item in the new Gear button is almost cut on Italian build (see screenshot).

Also, it's not completely safe to use text-transform: uppercase (see bug 830002), and some locales prefer to avoid it.

Comment 1

[Ed Lee :Mardak]

How should we go about fixing the uppercase menu title for Aurora 40?

http://mxr.mozilla.org/mozilla-aurora/source/browser/locales/en-US/chrome/browser/newTab.dtd#9

Do we add a localization note in 40 about it being uppercase. But it's probably not end of the world if we took out transform uppercase in 40 and only added the note in 41.. ?

- <!ENTITY newtab.customize.cog.title "New Tab Controls">
+ <!ENTITY newtab.customize.cog.title "NEW TAB CONTROLS">

Comment 2

[Francesco Lodolo [:flod]]

I wouldn't touch the string in Fx40 at this point, but we can add a comment explaining that the string will be transformed.

We can fix this on Fx41 (with a new string ID).

Comment 3

[Théo Chevalier]

Attachment: Missing right padding

Adding it here since it's related, there's also a missing right padding when title is the longest string.

Comment 4

[Ed Lee :Mardak]

Bug 1158859 introduced the menu changes for Nightly 40, so we'll need to uplift sizing fixes and potentially the removal of text-transform.

Comment 5

[Marina Samuel [:emtwo]]

Francesco, could you please test out the build here and let me know if it looks good to you? 

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5c1fa2b071d5

This build adds right padding to the cog menu so no truncating occurs. It also adjusts the intro cog menu issues and static intro button widths mentioned in bug 1165394.

The build also includes removal of text-transform for uppercase (only for fx41)

Thanks!

Comment 6

[Francesco Lodolo [:flod]]

Attachment: large menu.png

It seems to work fine, but I'd probably a maximum width limit, after which text should start wrapping.

Comment 7

[Francesco Lodolo [:flod]]

Any update? We currently look pretty broken on Developer Edition because of this issue.

Comment 8

[Marina Samuel [:emtwo]]

I'll have the patches ready for review later today.

Comment 9

[Marina Samuel [:emtwo]]

Attachment: Part 1: v1: Padding & text wrap fixes

Comment 10

[Marina Samuel [:emtwo]]

Attachment: Part 2a: v1 (fx-40): Add a comment for text transform uppercase

Comment 11

[Marina Samuel [:emtwo]]

Attachment: Part 2b: v1 (fx-41): Remove text transform uppercase

Comment 12

[Marina Samuel [:emtwo]]

Started up a try build here for testing: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f9563ccd9124

Comment 13

[Ed Lee :Mardak]

Comment on attachment 8612649 [details] [diff] [review]
Part 1: v1: Padding & text wrap fixes

>+++ b/browser/base/content/newtab/newTab.css
> #newtab-customize-title {
>   color: #7A7A7A;
>   font-size: 14px;
>   background-color: #FFFFFF;
>-  height: 52px;
>-  line-height: 52px;
>-  padding-left: 15px;
>+  line-height: 25px;
>+  padding: 15px;
>   font-weight: 600;
>   text-transform: uppercase;
>   cursor: default;
>   border-radius: 5px 5px 0px 0px;
>+  max-width: 300px;
This max width will prevent the title panel item from stretching as much as the other panel items. This means the border bottom will only be 300px wide.

> .newtab-customize-panel-item:not(:last-child),
> .newtab-search-panel-engine {
>   border-bottom: 1px solid threedshadow;
Maybe we can draw the border at the top except for the first child (i.e., title line)?

> #newtab-intro-modal {
>   font-family: "Helvetica";
>-  width: 700px;
>   height: 500px;
>   position: fixed;
>   left: 0;
>   right: 0;
>   top: 0;
>   bottom: 0;
>   margin: auto;
>   background: linear-gradient(#FFFFFF, #F9F9F9);
>   box-shadow: 0px 2px 4px rgba(0, 0, 0, 0.7);
>   border-radius: 8px 8px 0px 0px;
>+  min-width: 715px;
>+  position: relative;
>+  max-width: 75%;
>+  display: inline-block;
>+  transform: translateY(50%);
I believe the vertically center trick you're looking for is
top: 50%;
transform: translateY(-50%);

> #newtab-intro-header {
>   font-size: 28px;
>   color: #737980;
>   text-align: center;
>   top: 50px;
>   position: relative;
>   border-bottom: 2px solid #E0DFE0;
>   padding-bottom: 10px;
>-  width: 600px;
This could be the longest line, so we'll want padding on sides so the text doesn't extend directly to the edge.

Comment 15

[Francesco Lodolo [:flod]]

(we should really land this)

Comment 16

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/8ed8f05c29c0

Comment 17

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/8ed8f05c29c0

Comment 18

[Ed Lee :Mardak]

We'll split off the text padding issue.

Comment 19

[Marina Samuel [:emtwo]]

Comment on attachment 8612650 [details] [diff] [review]
Part 2a: v1 (fx-40): Add a comment for text transform uppercase

Approval Request Comment

[Feature/regressing bug #]: bug 1158859

[User impact if declined]: localization may be incorrect due to css uppercase transformation.

[Describe test coverage new/current, TreeHerder]: N/A - just adding a comment

[Risks and why]: no risk - just a comment

[String/UUID change made/needed]: N/A

Comment 20

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8612650 [details] [diff] [review]
Part 2a: v1 (fx-40): Add a comment for text transform uppercase

Comment only which will simplify the life of our localizer. Taking it.

Comment 21

[Marina Samuel [:emtwo]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/558caff6816e

Comment 22

[Cornel Ionce [:noni] [Hubs QA]]

Verified fixed on Firefox 40.0b2 (build ID:20150817163452) and Firefox 40.0.2 (build ID: 20150812163655) using the Fr/It locales.