Closed Bug 1236372 Opened 4 years ago Closed 4 years ago

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

Categories

(Firefox :: Sync, defect, P1)

defect

Tracking

()

VERIFIED FIXED
Firefox 47
Tracking Status
firefox45 --- verified
firefox46 --- verified
firefox47 --- verified

People

(Reporter: aryx, Assigned: markh, Mentored)

References

Details

Attachments

(3 files)

Attached image 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.)
Blocks: 1201331
Flags: firefox-backlog+
Mentor: markh
Whiteboard: [good first bug][lang=css]
Blocks: 1239084
Attached image 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?
Flags: needinfo?(rfeeley)
See Also: → 1224412
Priority: -- → P1
hello Markh, I am a newbee, I want to contribute in the open source.so please assign me this bug.
Definitely prefer without scrollbars if auto-height is not possible.
Flags: needinfo?(rfeeley)
(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.
Assignee: nobody → markh
Flags: needinfo?(aryx.bugmail)
Whiteboard: [good first bug][lang=css]
Thank you, changing it to 33em solves the issue on both Nightly and Aurora.
Flags: needinfo?(aryx.bugmail)
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.
Attachment #8716188 - Flags: review?(gijskruitbosch+bugs)
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.
Attachment #8716188 - Flags: review?(gijskruitbosch+bugs) → review+
See Also: → 1248506
(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 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
Attachment #8716188 - Flags: approval-mozilla-beta?
Attachment #8716188 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/49adf1dee4dd
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 47
Flags: qe-verify+
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
Attachment #8716188 - Flags: approval-mozilla-beta?
Attachment #8716188 - Flags: approval-mozilla-beta+
Attachment #8716188 - Flags: approval-mozilla-aurora?
Attachment #8716188 - Flags: approval-mozilla-aurora+
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?
Flags: needinfo?(aryx.bugmail)
Fixed with Firefox 45.0 beta 9 on Windows 8.1 (125% dpi).
Status: RESOLVED → VERIFIED
Flags: needinfo?(aryx.bugmail)
Thanks Sebastian for your testing! Updating flags to reflect the current situation.
Depends on: 1251842
You need to log in before you can comment on or make changes to this bug.