1198287 - New widths for the two sections of about:privatebrowsing

Status: VERIFIED FIXED

(Firefox :: Private Browsing, defect, P1)

Description

[:Paolo Amadini]

The section of about:privatebrowsing about private browsing should be slightly larger in proportion than the section about Tracking Protection.

We may not be able to uplift this to Firefox 42 because the widths are localized.

Comment 1

[:Paolo Amadini]

Attachment: MozReview Request: Bug 1198287 - New widths for the two sections of about:privatebrowsing. r=bgrins

Bug 1198287 - New widths for the two sections of about:privatebrowsing. r=bgrins

Comment 2

[:Paolo Amadini]

This change was requested as part of visual design improvements.

One possible concern with this change is that on smaller screens with the default window size (not fullscreen) we may not have enough horizontal space for both sections. However, that's also true of localized version that require more space anyways. We already wrap the two sections in that case.

Comment 3

[Tim Nguyen :ntim]

Comment on attachment 8652352 [details]
MozReview Request: Bug 1198287 - New widths for the two sections of about:privatebrowsing. r=bgrins

Can we be smarter about calculating those widths ? Using localized widths is definitively a hack. Moreover, the wanted width may be different depending on the platform. Different localizers use different platforms too, so it's hard to know if we'll get the wanted result everywhere.

Comment 4

[Brian Grinstead [:bgrins]]

(In reply to Tim Nguyen [:ntim] (mostly away until 26 August) from comment #3)
> Comment on attachment 8652352 [details]
> MozReview Request: Bug 1198287 - New widths for the two sections of
> about:privatebrowsing. r=bgrins
> 
> Can we be smarter about calculating those widths ? Using localized widths is
> definitively a hack. Moreover, the wanted width may be different depending
> on the platform. Different localizers use different platforms too, so it's
> hard to know if we'll get the wanted result everywhere.

I agree - I think we can solve this with flexbox.  I will see if I can get that working

Comment 5

[Brian Grinstead [:bgrins]]

Attachment: MozReview Request: Bug 1198287 - Update flexbox usage for about:privatebrowsing to get rid of hardcoded localized widths;r=paolo

Bug 1198287 - Update flexbox usage for about:privatebrowsing to get rid of hardcoded localized widths;r=paolo

Comment 6

[Brian Grinstead [:bgrins]]

(In reply to Brian Grinstead [:bgrins] from comment #5)
> Created attachment 8652589 [details]
> MozReview Request: Bug 1198287 - Update flexbox usage for
> about:privatebrowsing to get rid of hardcoded localized widths;r=paolo
> 
> Bug 1198287 - Update flexbox usage for about:privatebrowsing to get rid of
> hardcoded localized widths;r=paolo

Paolo, please take a look at this alternate approach.  I think it handles most cases similarly to how the original way does, except that the width of the privateBrowsingSection and trackingProtectionSection are flexible (with the restriction that they will never get smaller than their headers or the forgotten/kept list).

Pros are that localizers don't need to pick a harcoded width and it should accomodate different system fonts / platforms.  On the other hand, there's less control over exactly how the design ends up rendered, i.e. it's going to render differently based on the size of the screen (right now it will take up to 80% of the screen size and then flow after that happens).

Comment 7

[Brian Grinstead [:bgrins]]

If you don't like the alternate approach let's discuss further to come up with a plan for the best way to handle this

Comment 8

[:Paolo Amadini]

Comment on attachment 8652589 [details]
MozReview Request: Bug 1198287 - Update flexbox usage for about:privatebrowsing to get rid of hardcoded localized widths;r=paolo

(In reply to Brian Grinstead [:bgrins] from comment #7)
> If you don't like the alternate approach let's discuss further to come up
> with a plan for the best way to handle this

I did actually experiment with several alternate approaches to localized widths, and after exploring what's technically possible with CSS (which isn't much, CSS is definitely not suited for any serious responsive design work) I discussed our options with Bryan.

The reality is that every language has different layout needs and the aesthetical decision about it cannot be automated that easily. For example I tried to make the assumption that the Tracking Protection title and indicator won't wrap (I have a patch that handles that locally) but that just doesn't work for some languages with very long words. On the other hand CJK scripts may need more space around the titles and sentences, but not too much margin like a stretching approach would give. Also the sentences that can wrap must keep a limited line length for legibility.

I also tried using flexbox or something else to keep the desired proportional ratio between the sections but that didn't work very well.

Obviously we could write some fairly complicated JavaScript with input form the needs of each locale but we could easily spend more time on it than our localizers double-checking the widths. For what it's worth I searched the l10n repositories before writing this patch and at the moment the larger width for the Private Browsing section was Italian with 29em, which is still less than the 30em set by this updated patch, so at least I don't expect that most Latin languages will need to change that value again.

There is one very small improvement still possible for calculating the width of the enabled indicator automatically using a XUL stack. I have a patch for that too, but since the current indicator has already been localized and we're considering to move it to a different style and position anyways, I don't think it's worth changing.

There's been a lot of work trying to reconcile the different needs here - this design is unlike anything we have done before. Unfortunately most of it consisted in me doing research and discarding options before even bringing them up in order to save other people's time, so it is not too visible. We should definitely discuss this again with you as maybe we can come up with something better by using CSS or JavaScript techniques I didn't consider.

Comment 9

[:Paolo Amadini]

(In reply to Tim Nguyen [:ntim] (mostly away until 26 August) from comment #3)
> the wanted width may be different depending
> on the platform. Different localizers use different platforms too, so it's
> hard to know if we'll get the wanted result everywhere.

By the way, that's definitely a challenge we considered - it would be great if we could make these decisions automatically. However, so far the results of this manual approach look good, and even small mismatches won't significantly break the layout. For example, in English, OS X 10.9 and 10.10 have different layouts because of different fonts, but both look good based on Bryan's visual design feedback.

Comment 10

[Brian Grinstead [:bgrins]]

(In reply to :Paolo Amadini from comment #8)
> Comment on attachment 8652589 [details]
> MozReview Request: Bug 1198287 - Update flexbox usage for
> about:privatebrowsing to get rid of hardcoded localized widths;r=paolo
> 
> (In reply to Brian Grinstead [:bgrins] from comment #7)
> > If you don't like the alternate approach let's discuss further to come up
> > with a plan for the best way to handle this
> 
> I did actually experiment with several alternate approaches to localized
> widths, and after exploring what's technically possible with CSS (which
> isn't much, CSS is definitely not suited for any serious responsive design
> work) I discussed our options with Bryan.
> 
> The reality is that every language has different layout needs and the
> aesthetical decision about it cannot be automated that easily. For example I
> tried to make the assumption that the Tracking Protection title and
> indicator won't wrap (I have a patch that handles that locally) but that
> just doesn't work for some languages with very long words. On the other hand
> CJK scripts may need more space around the titles and sentences, but not too
> much margin like a stretching approach would give. Also the sentences that
> can wrap must keep a limited line length for legibility.
> 
> I also tried using flexbox or something else to keep the desired
> proportional ratio between the sections but that didn't work very well.
> 
> Obviously we could write some fairly complicated JavaScript with input form
> the needs of each locale but we could easily spend more time on it than our
> localizers double-checking the widths. For what it's worth I searched the
> l10n repositories before writing this patch and at the moment the larger
> width for the Private Browsing section was Italian with 29em, which is still
> less than the 30em set by this updated patch, so at least I don't expect
> that most Latin languages will need to change that value again.
> 
> There is one very small improvement still possible for calculating the width
> of the enabled indicator automatically using a XUL stack. I have a patch for
> that too, but since the current indicator has already been localized and
> we're considering to move it to a different style and position anyways, I
> don't think it's worth changing.

Since the localized widths are already in place, I don't think there's any harm in proceeding with a patch that updates the values like you have.

> There's been a lot of work trying to reconcile the different needs here -
> this design is unlike anything we have done before.  Unfortunately most of it
> consisted in me doing research and discarding options before even bringing
> them up in order to save other people's time, so it is not too visible. We
> should definitely discuss this again with you as maybe we can come up with
> something better by using CSS or JavaScript techniques I didn't consider.

Since this is the first time we are doing a design like this, it's likely to end up being copied if we do something similar in the future. So worth further discussion definitely.

Comment 11

[Brian Grinstead [:bgrins]]

Comment on attachment 8652352 [details]
MozReview Request: Bug 1198287 - New widths for the two sections of about:privatebrowsing. r=bgrins

https://reviewboard.mozilla.org/r/17133/#review15395

::: browser/locales/en-US/chrome/browser/aboutPrivateBrowsing.dtd:13
(Diff revision 1)
> +     lenght of the headers and text, but should be roughly 1.5 times the width

lenght -> length

::: browser/locales/en-US/chrome/browser/aboutPrivateBrowsing.dtd:47
(Diff revision 1)
> -<!ENTITY trackingProtection.width              "22em">
> +<!ENTITY trackingProtection.width1             "22em">

Do we need to update this entity name?  The value hasn't changed

Comment 12

[:Paolo Amadini]

https://reviewboard.mozilla.org/r/17133/#review15499

::: browser/locales/en-US/chrome/browser/aboutPrivateBrowsing.dtd:41
(Diff revision 1)
>  <!-- LOCALIZATION NOTE (trackingProtection.width):

I'll update the entity name in the note as well.

Comment 13

[:Paolo Amadini]

https://reviewboard.mozilla.org/r/17133/#review15395

> Do we need to update this entity name?  The value hasn't changed

We'd like localizers to look at this again since wrapping the header is fine, now indicated in the localization note.

Comment 14

[Pulsebot]

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

Comment 15

[:Paolo Amadini]

Okay, so this is a patch touching localizable entities, but not strings, with potential for uplift.

We did this because we discussed the design in depth and better defined what to do when the length of the text changes. I updated the localization notes accordingly, see here for the latest version:

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

We would like localizers to review the widths at least on Nightly, but I'm unsure about the relative importance of this for the Firefox 42 launch. This decision should be made by product and release management, but I'd like to see input from localization teams on the impact as well.

Here you can see which locales customized the widths:

http://mxr.mozilla.org/l10n-mozilla-aurora/search?string=aboutPrivateBrowsing.width+
http://mxr.mozilla.org/l10n-mozilla-aurora/search?string=trackingProtection.width+

Sample localized Developer Edition builds can be downloaded from here:

http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2015-08-26-00-40-06-mozilla-aurora-l10n/

In French, for example, at present on a default sized window the sections wrap one under the other because the title in the Tracking Protection section doesn't wrap (following the previous instructions in the localization notes).

An alternative to uplift is to reach out to localizers specifically about this change. Francesco, any input on the preferred approach?

Note that we should uplift bug 1193806 in any case before this patch, as the width is influenced by the font.

Comment 16

[Francesco Lodolo [:flod]]

(In reply to :Paolo Amadini from comment #15)

> http://mxr.mozilla.org/l10n-mozilla-aurora/
> search?string=aboutPrivateBrowsing.width+
> http://mxr.mozilla.org/l10n-mozilla-aurora/search?string=trackingProtection.
> width+

mxr is not really useful in these cases
https://transvision.mozfr.org/string/?entity=browser/chrome/browser/aboutPrivateBrowsing.dtd:aboutPrivateBrowsing.width&repo=aurora
https://transvision.mozfr.org/string/?entity=browser/chrome/browser/aboutPrivateBrowsing.dtd:trackingProtection.width&repo=aurora

> Okay, so this is a patch touching localizable entities, but not strings,
> with potential for uplift.

It's still breaking string freeze, so we shouldn't do that on mozilla-aurora, even if falling back to English values could be OK in these cases.

It's also scary that we end up landing a redesign so late in the aurora cycle. Is it possible to see a before/after screenshot, and how these values affect the layout?

Personally I'm not a huge fan of having widths in localizable files, unless it's a last resort: most locales will simply copy the English values, often you just put in a value "reasonable on paper" (based on how longer your strings are compared to en-US) and try to test.

I think Pike is trying to get some automations to create screenshots for that specific window, but I don't know how far he got or plans to go, e.g. https://github.com/Pike/firefox-l10n-tests/blob/master/it/darwin/privacy/welcome.png

That could help if we land the design without updating the IDs, asking localizers to double check the results.

Comment 17

[:Paolo Amadini]

(In reply to Francesco Lodolo [:flod] from comment #16)
> It's also scary that we end up landing a redesign so late in the aurora
> cycle. Is it possible to see a before/after screenshot, and how these values
> affect the layout?

The most interesting cases would be locale dependent, I can attach an example for English to see the difference in proportion.

> Personally I'm not a huge fan of having widths in localizable files, unless
> it's a last resort: most locales will simply copy the English values, often
> you just put in a value "reasonable on paper" (based on how longer your
> strings are compared to en-US) and try to test.

Yeah, on the other hand there are no good defaults we could use that would work everywhere. I think the new English defaults set here will work in many cases without changes, but having a localizable entity will give the flexibility of changing these if an issue with a specific locale arises later in the cycle.

> I think Pike is trying to get some automations to create screenshots for
> that specific window, but I don't know how far he got or plans to go, e.g.
> https://github.com/Pike/firefox-l10n-tests/blob/master/it/darwin/privacy/
> welcome.png

That's great! We talked about this idea, but were unsure how much effort it was to implement - it's good to see it has been done to some extent already. One thing we'd like to test are different window sizes.

> That could help if we land the design without updating the IDs, asking
> localizers to double check the results.

So, you're suggesting we could update the widths just for English without changing the entity names in the Developer Edition channel, ask localizers to compare that with the screenshot or local version for their language, and adjust if needed?

Comment 18

[:Paolo Amadini]

Attachment: Old version, English, OS X 10.10 fonts

Comment 19

[:Paolo Amadini]

Attachment: New version, English, OS X 10.10 fonts

Comment 20

[:Paolo Amadini]

Attachment: New version, English, OS X 10.9 fonts

This last screenshot shows that it is fine for the enabled or disabled indicator to wrap with certain fonts or languages.

Comment 21

[:Paolo Amadini]

Some locales like French opted for a vertical layout:

https://raw.githubusercontent.com/Pike/firefox-l10n-tests/55a2c6c8/gd/darwin/privacy/welcome.png

(French was not in the available screenshots.)

I think the horizontal layout with the titles wrapping is now preferred.

Comment 22

[Francesco Lodolo [:flod]]

(In reply to :Paolo Amadini from comment #17)
> So, you're suggesting we could update the widths just for English without
> changing the entity names in the Developer Edition channel, ask localizers
> to compare that with the screenshot or local version for their language, and
> adjust if needed?

Yes (updating comments and values, but not string IDs), but a lot depends on how likely are things to break if some locales keep the existing values. 

How is the third screenshot obtained? If I understand this correctly: if the width is too small, we practically fall back to the previous design with label and switch on two lines. Is that correct?

For example, how does English look with the previous value (25em) for aboutPrivateBrowsing.width?

One potential issue is if "ON" is a lot shorter than "OFF" (or viceversa), we'd end up displaying a somehow incoherent design (one line in one case, two lines in the other).

Comment 23

[:Paolo Amadini]

(In reply to Francesco Lodolo [:flod] from comment #22)
> Yes (updating comments and values, but not string IDs), but a lot depends on
> how likely are things to break if some locales keep the existing values. 

For now only a few locales, like French, look totally different from what we expect. The proportions will be wrong on most locales, but I'm not sure how much weight that has for the launch.

> How is the third screenshot obtained?

It's with the default fonts on OS X 10.9. The others are with the fonts changed to match the default on OS X 10.10. The intent is for the two "new version" screenshots to represent the same source code revision rendered on two different platforms.

> If I understand this correctly: if the
> width is too small, we practically fall back to the previous design with
> label and switch on two lines. Is that correct?

To clarify, wrapping when the width is too small is the latest decision we made on the design, not a fallback, but it's correct that we may have both. The title itself can be on two lines with the last word and the indicator both on the second line.

> For example, how does English look with the previous value (25em) for
> aboutPrivateBrowsing.width?

That's the "old version" screenshot.

> One potential issue is if "ON" is a lot shorter than "OFF" (or viceversa),
> we'd end up displaying a somehow incoherent design (one line in one case,
> two lines in the other).

We have a third width entity for the indicator at present, in order to avoid this issue. Note that in future revisions we'll likely move this indicator to another place, and we'll be able to set the width to the largest text automatically without the need for the entity.

Comment 24

[:Paolo Amadini]

So there are two things we want to do here, one is to adjust the proportions, and the other is to fix the locales that have a very different layout than the expected one because titles don't wrap, like French.

Comment 25

[Axel Hecht [:Pike]]

Note, I can help you run the tests, but it's a bit funky with like wget's and awk and a branch and installing mozprofile from mozilla-central :-/

I did find an interesting artifact with today's aurora build, while trying to debug gd.

If you set the width to 25 or 26em on both divs, and then horizontally resize, you see the following behavior:

1) horizontal big-font
2) vertical big-font
3) horizontal small-font
4) vertical small-font

I suspect that 2) should have never happened?

Comment 26

[:Paolo Amadini]

(In reply to Axel Hecht [:Pike] from comment #25)
> I suspect that 2) should have never happened?

I think it's fine if that happens, we don't have much power over it with CSS media queries anyways.

Comment 27

[:Paolo Amadini]

So, unless I'm misunderstanding some aspect, the plan of record for Firefox 42 could be:
 - Update the proportions for English without breaking string freeze
 - Reach out to localizers asking to double check the widths with the new requirements
 - Use Axel's screenshot tool for verification

For Firefox 43, we recover any locale we missed above because we changed the entity name.

Comment 28

[Francesco Lodolo [:flod]]

(In reply to :Paolo Amadini from comment #27)
> So, unless I'm misunderstanding some aspect, the plan of record for Firefox
> 42 could be:
>  - Update the proportions for English without breaking string freeze
>  - Reach out to localizers asking to double check the widths with the new
> requirements
>  - Use Axel's screenshot tool for verification
> 
> For Firefox 43, we recover any locale we missed above because we changed the
> entity name.

The plan sounds good to me, the sooner it happens the better, to give a chance to localizers to test properly.

Note that there's also a third option, which is "don't touch anything on aurora", but I don't know if it's been already ruled out and who's the owner for such decision ;-)

Comment 29

[:Paolo Amadini]

(In reply to Francesco Lodolo [:flod] from comment #28)
> The plan sounds good to me, the sooner it happens the better, to give a
> chance to localizers to test properly.
> 
> Note that there's also a third option, which is "don't touch anything on
> aurora", but I don't know if it's been already ruled out and who's the owner
> for such decision ;-)

Some locales look broken according to our visual design review, so we'll need some action there anyways. To answer the question, the owners for this decision are Javaun and Lawrence (product and release management). I'm meeting with Javaun in two hours so I'll recap and we'll have an answer :-)

Comment 30

[Axel Hecht [:Pike]]

The "default size window" is depending on the hardware of your currently selected monitor. I.e., before I hard-coded the screenshot size, they would flip depending on whether the test started with my laptop focused or my 24inch portrait mode monitor focused.

I'm not sure if we should use that as a criteria, unless we can be more specific about that. If we have a recommended window size, we should transform that into a strict recommendation on maximum values for the two widths. Let's not have 100 folks test on three or four platforms to figure this out, when we can just do that once.

Comment 31

[:Paolo Amadini]

According to http://www.w3schools.com/browsers/browsers_display.asp, I guess a horizontal resolution of 1280 and a vertical resolution of 768 are the worst case we have to handle. For lower resolutions, a less optimal but still usable wrapping design can be acceptable. Not sure what the default window size (not maximized) for those cases is - we may also assume users on resolutions like 1280x1024 and 1024x768 will use the browser maximized.

Comment 32

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/d942461af575

Comment 33

[:Paolo Amadini]

We're going ahead with the plan, I'll see if I can create a patch for Developer Edition tomorrow.

Comment 34

[Paul Silaghi, QA [:pauly]]

I've tried some nightly locales:
de, es-ar, fr, it, ja-jp, ru, zh-cn - look ok
ar - http://i.imgur.com/yHCxn1u.png
Thoughts?

Comment 35

[:Paolo Amadini]

(In reply to Paul Silaghi, QA [:pauly] from comment #34)
> ar - http://i.imgur.com/yHCxn1u.png
> Thoughts?

The bold effect is just the different font for the Latin text, some strings have not been translated yet.

Can you file a bug for the LTR issue where the ticks are on the wrong side? (Also, not sure if we need to flip them.) Thanks for catching the bug!

Comment 36

[Paul Silaghi, QA [:pauly]]

Filed bug 1200661 for the RTL issue.
Marking this bug as verified fixed.