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

VERIFIED FIXED in Firefox 45

Status

()

Firefox
Sync
P1
normal
VERIFIED FIXED
2 years ago
2 years ago

People

(Reporter: aryx, Assigned: markh, Mentored)

Tracking

(Depends on: 1 bug, Blocks: 1 bug)

unspecified
Firefox 47
Points:
---
Dependency tree / graph
Bug Flags:
firefox-backlog +

Firefox Tracking Flags

(firefox45 verified, firefox46 verified, firefox47 verified)

Details

Attachments

(3 attachments)

Created attachment 8703451 [details]
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.)
(Assignee)

Updated

2 years ago
Blocks: 1201331
Flags: firefox-backlog+
(Assignee)

Updated

2 years ago
Mentor: markh@mozilla.com
Whiteboard: [good first bug][lang=css]
Blocks: 1239084
(Assignee)

Comment 1

2 years ago
Created attachment 8707295 [details]
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)
(Assignee)

Updated

2 years ago
See Also: → bug 1224412
Priority: -- → P1

Comment 2

2 years ago
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)
(Assignee)

Comment 4

2 years ago
(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)
(Assignee)

Comment 6

2 years ago
Created attachment 8716188 [details] [diff] [review]
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.
Attachment #8716188 - Flags: review?(gijskruitbosch+bugs)

Comment 7

2 years ago
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+
(Assignee)

Updated

2 years ago
See Also: → bug 1248506
(Assignee)

Comment 8

2 years ago
(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.
(Assignee)

Comment 10

2 years ago
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?

Comment 11

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/49adf1dee4dd
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox47: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 47
status-firefox45: --- → affected
status-firefox46: --- → affected
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+

Comment 13

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/b50adf51c34d
status-firefox45: affected → fixed

Comment 14

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/43172e589b99
status-firefox46: affected → fixed
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.
status-firefox45: fixed → verified
status-firefox46: fixed → verified
status-firefox47: fixed → verified
Flags: qe-verify+

Updated

2 years ago
Depends on: 1251842
You need to log in before you can comment on or make changes to this bug.