1010537 - [fr] Transition away from using style.maxWidth to adjust the UI

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Browser, defect)

Description

[Zibi Braniecki [:zbraniecki][:gandalf]]

Spin-off from bug 994290.

French translation of Browser currently uses style.maxWidth attribute on three entities to adjust UI.

We'd like to clean the attribute setting in l10n.js and we consider blanked support for style to be not in line with our security policy (e.g. style.display='none' or style.content would work).

Comment 1

[Zibi Braniecki [:zbraniecki][:gandalf]]

Attachment: maxWidth impact on french tabs on Keon

The screenshot shows the difference in truncation between french browser with and without style.maxWidth.

Comment 2

[Théo Chevalier]

Hopefully, Rocketbar will land on 2.0, so this should not be a problem

Comment 3

[Zibi Braniecki [:zbraniecki][:gandalf]]

Theo: right, except that I'd prefer not to wait for us to phase out the Browser app until we can fix that. It blocks us with a lot of work on l10n.js that we want to get in for 2.0.

Ben, Dale, do you have any ideas if we could improve CSS to not require that hack?

Comment 4

[Théo Chevalier]

Since we need a real temp hack until Rocketbar, we could just what I'm doing, but in the Browser App CSS stylesheet:

html[lang="fr"] tab1-element {
   max-width: 15%;
}

html[lang="fr"] tab2-element {
   max-width: 27%;
}

etc.
something like that I guess?

Comment 5

[Zibi Braniecki [:zbraniecki][:gandalf]]

I would love to avoid language specific solution here. Is it possible to improve CSS so that it flexes properly?

Comment 6

[Dale Harvey (:daleharvey)]

This has been annoying for ages and doesnt look too tricky, will give it a shot

Comment 7

[Dale Harvey (:daleharvey)]

Attachment: https://github.com/mozilla-b2g/gaia/pull/19350

Its impossible to have both consistent padding and not cut these off at the edges, this will cut off the start of the first tab and end of the last, but I think thats a reasonable tradeoff to get rid of all the cross language issues we have been having with this.

Comment 8

[Dale Harvey (:daleharvey)]

I tested against a french locale, no overflow

Comment 9

[Ben Francis [:benfrancis]]

Comment on attachment 8424436 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/19350

This CSS has a complex history and whenever we change it we seem to break some locales. I'd be happier if we could test this in more than just French before landing. It doesn't look right in one of the RTL languages for example, and I'm not sure whether this problem occurs for some LTR languages as well :S

It might also be worth getting UX input on some screenshots as to whether this is considered an improvement.

Comment 10

[Dale Harvey (:daleharvey)]

Comment on attachment 8424436 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/19350

This layout cannot be implemented to scale / flex properly while giving consistent padding, at least not in any reasonable way, lets just live with the hack we know works until the browser dies.

Used the technique from https://bugzilla.mozilla.org/show_bug.cgi?id=1010537#c4 to just move the locales to the css (the extra selector is needed for specificity)

Comment 11

[Ben Francis [:benfrancis]]

Comment on attachment 8424436 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/19350

This is better than setting styles in .properties files and has a lower risk of regressions than the previous patch, so r+me given this should be a temporary hack that is no worse than we already shipped.

Thanks Dale.

Comment 12

[Ben Francis [:benfrancis]]

I restarted the integration tests on Travis, there was a failing homescreen 2.0 test on Travis which seems unrelated, not sure if this is a known intermittent.

 1) Vertical - Bookmark Bookmarking appends to last group:

AssertionError: 46 == 45

at Context.<anonymous> (/home/travis/build/mozilla-b2g/gaia/dev_apps/home2/test/marionette/bookmark_test.js:54:12)

at callFn (/home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runnable.js:223:21)

at Test.Runnable.run (/home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runnable.js:216:7)

at Runner.runTest (/home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runner.js:374:10)

at /home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runner.js:452:12

at next (/home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runner.js:299:14)

at /home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runner.js:309:7

at next (/home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runner.js:247:23)

at /home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runner.js:271:7

at done (/home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runnable.js:185:5)

at callFn (/home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runnable.js:228:7)

at Hook.Runnable.run (/home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runnable.js:216:7)

at next (/home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runner.js:259:10)

at /home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runner.js:271:7

at done (/home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runnable.js:185:5)

at /home/travis/build/mozilla-b2g/gaia/node_modules/mocha/lib/runnable.js:199:9

at Object.executeHook (/home/travis/build/mozilla-b2g/gaia/node_modules/marionette-client/lib/marionette/client.js:369:18)

at process._tickCallback (node.js:415:13)

Comment 13

[Dale Harvey (:daleharvey)]

Cheers, https://github.com/mozilla-b2g/gaia/commit/c320ddbf2cc2a484f5f54396a86a96bcb071dc94

Comment 14

[Théo Chevalier]

Fixed in fr l10n repo as well https://hg.mozilla.org/gaia-l10n/fr/rev/ae185875ede9

Comment 15

[Zibi Braniecki [:zbraniecki][:gandalf]]

Thanks a lot for help with this guys!