961375 - Label for "Hide title bar when scrolling" pref is cropped in portrait mode

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, enhancement)

Description

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Latest Firefox Mobile Nightly and Aurora on Android 4.1.2

The preference""Hide title bar when scrolling" currently gets cropped in portrait mode. If the state gets negate and the preference controls showing and not hiding of the titlebar, the label would fit into the available space.

Comment 1

[Aaron Train [:aaronmt]]

Which device is it cropped for you? Can you attach a screenshot?

Comment 2

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: screenshot of issue

It's even cropped for en-US on Nexus S.

Comment 3

[:Margaret Leibovic]

I wonder if we could come up with a shorter title for the pref, and then use the small description text underneath to clarify what the preference does.

Comment 4

[Aaron Train [:aaronmt]]

Yeah Chrome uses a lot of android:summary for their setting and their labels are quite short

Comment 5

[Ian Barlow (:ibarlow)]

Gah, sorry for the delay. 

What if we just change the string to "Scroll title bar"?

Comment 6

[:Margaret Leibovic]

Attachment: patch

That sounds like a fine solution to me.

Comment 7

[:Margaret Leibovic]

flod, is this the type of change we would uplift for a late-l10n fix, or should we just let it ride the trains? I was debating whether or not to just make this an en-US fix, but I worry that localizers might have equally long labels in their locales.

Comment 8

[Francesco Lodolo [:flod]]

I'd say:
1. Let the patch adding a new string ride the trains, it's way too late for Aurora
2. Fix only the en-US string (no new ID, just update the string content) on Aurora/Beta and send a warning on dev-l10n about this issue, so that localizers can take action is they want. A string cut only in portrait mode is better than an English string everywhere, in case they don't have time to localize it.

Ideally they should have caught it during QA, some locales did for sure (e.g. French)
http://transvision.mozfr.org/string/?entity=mobile/android/base/android_strings.dtd:pref_dynamic_toolbar&repo=central

And that reminds me that I should test on a phone, not just a tablet...

Comment 9

[:Margaret Leibovic]

Thanks for the update. Just landed the regular patch on fx-team:
https://hg.mozilla.org/integration/fx-team/rev/ba6a785bf7ea

I can whip up a patch to change only the en-US string on Aurora.

Comment 10

[:Margaret Leibovic]

Attachment: en-US update for aurora

Comment 11

[:Margaret Leibovic]

Comment on attachment 8367370 [details] [diff] [review]
en-US update for aurora

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 944925
User impact if declined: title for setting is cut off on some devices
Testing completed (on m-c, etc.): tested locally
Risk to taking this patch (and alternatives if risky): low-risk, just a string update for en-US
String or IDL/UUID changes made by this patch: consulted with flod, this patch just updates the en-US string, which won't affect localizers, but I will make a post to dev-l10n explaining the issue in case they want to update their locales

Comment 12

[Francesco Lodolo [:flod]]

Comment on attachment 8367370 [details] [diff] [review]
en-US update for aurora

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

Patch is good for me, thank Margaret.

Comment 13

[bhavana bajaj [:bajaj]]

Waiting for fx-team/central landing before approving on branch

Comment 14

[Ryan VanderMeulen [:RyanVM]]

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

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/66bbad1eb010

Comment 16

[Catalin Suciu [:csuciu]]

Verified fixed on FF release 28 and Firefox 29 Beta 4