1236372 - Synced Tabs panel should autosize to fit localized mobile promo content (scrollbars at the moment)

Status: VERIFIED FIXED

(Firefox :: Sync, defect, P1)

Description

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

Attachment: screenshot of issue

The German localization of the mobile Sync promo in Firefox Desktop has a vertical scrollbar at the moment. The panel should auto-size. (German localization hasn't been released yet.)

Comment 1

[Mark Hammond [:markh] [:mhammond]]

Attachment: taller-panel.png

Note that this bug only happens when the button is in the toolbar area rather than in the panel, and is due to bug 1224412. As a work-around, we've set a min-height of 30em for the panel - which was supposed to be big enough to handle all the "static" content without scrollbars - see https://dxr.mozilla.org/mozilla-central/rev/e790bba372f14241addda469a4bdb7ab00786ab3/browser/themes/shared/customizableui/panelUIOverlay.inc.css#722

30em already leaves alot of whitespace for English strings, so I'm a little reluctant to increase this too far given the implications of this bug seem small (it's a minor visual glitch and the user doesn't even need to scroll to hit the UI)

So, handball to Ryan - the attachment shows what the panel looks like with English strings and a min-height of 33em (which I *think* will remove the scrollbar in this bug - but possibly not for languages with even longer translations) - Ryan, are you OK with that, or would you prefer to see the scrollbars for some locales?

Comment 2

[Parul]

hello Markh, I am a newbee, I want to contribute in the open source.so please assign me this bug.

Comment 3

[Ryan Feeley [:rfeeley]]

Definitely prefer without scrollbars if auto-height is not possible.

Comment 4

[Mark Hammond [:markh] [:mhammond]]

(In reply to Parul from comment #2)
> hello Markh, I am a newbee, I want to contribute in the open source.so
> please assign me this bug.

Sorry Paul but work on this bug is already ongoing - I've removed "good first bug" - sorry for the confusion.

Sebastian, is it possible to test that changing https://dxr.mozilla.org/mozilla-central/rev/e790bba372f14241addda469a4bdb7ab00786ab3/browser/themes/shared/customizableui/panelUIOverlay.inc.css#722 to 33em solves your problem? If it's a PITA I can put a try run up.

Comment 5

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

Thank you, changing it to 33em solves the issue on both Nightly and Aurora.

Comment 6

[Mark Hammond [:markh] [:mhammond]]

Attachment: 0013-Bug-1236372-increase-the-size-of-the-synced-tabs-pan.patch

Oops, I dropped this on the floor.

This increases the hard-coded size of the panel by 3em - there's still a change other localizations will see the scrollbar, but German is fairly verbose and seems a reasonable yardstick.

Comment 7

[:Gijs (he/him)]

Comment on attachment 8716188 [details] [diff] [review]
0013-Bug-1236372-increase-the-size-of-the-synced-tabs-pan.patch

rs=me based on Aryx's comment.

... however, wouldn't it be better to use calc() ? The image in some of these panes is absolutely (pixel) sized, isn't it? So it should ideally be some combination of that and the em size needed for accompanying text etc.

Comment 8

[Mark Hammond [:markh] [:mhammond]]

(In reply to :Gijs Kruitbosch from comment #7)
> ... however, wouldn't it be better to use calc() ? The image in some of
> these panes is absolutely (pixel) sized, isn't it? So it should ideally be
> some combination of that and the em size needed for accompanying text etc.

Probably yeah - I'm going to land this as-is and I opened bug 1248506 to investigate that.

Comment 9

[Pulsebot]

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

Comment 10

[Mark Hammond [:markh] [:mhammond]]

Comment on attachment 8716188 [details] [diff] [review]
0013-Bug-1236372-increase-the-size-of-the-synced-tabs-pan.patch

Approval Request Comment
[Feature/regressing bug #]: Synced Tabs
[User impact if declined]: Some locales may see scrollbars in the synced tabs panel due to localized text being longer than expected.
[Describe test coverage new/current, TreeHerder]: Existing tests pass.
[Risks and why]: Very low risk, 1 line CSS change limited to synced tabs styles.
[String/UUID change made/needed]: None

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/49adf1dee4dd

Comment 12

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8716188 [details] [diff] [review]
0013-Bug-1236372-increase-the-size-of-the-synced-tabs-pan.patch

Improve the support for other locales and low risk, taking it 
Should be in 45 beta 7

Comment 13

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

https://hg.mozilla.org/releases/mozilla-beta/rev/b50adf51c34d

Comment 14

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

https://hg.mozilla.org/releases/mozilla-aurora/rev/43172e589b99

Comment 15

[Vasilica Tamas, QA [:vtamas]]

Verified fixed on Firefox 47.0a1 (2016-02-22), Firefox 46.0a2 (2016-02-22) and Firefox 45 beta 8 (20160221141421) under Windows 10 64-bit and Mac OS X 10.9.5. Synced Tabs panel appears as intended and there are no scrollbars displayed for German localization.

Sebastian could you please also confirm that this bug is fixed?

Comment 16

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

Fixed with Firefox 45.0 beta 9 on Windows 8.1 (125% dpi).

Comment 17

[Vasilica Tamas, QA [:vtamas]]

Thanks Sebastian for your testing! Updating flags to reflect the current situation.