Aaron Benson Reporter
	Description • 6 years ago

After some investigation with Kate Hudson, this appears to only affect a small number of people and for no discernible reason. 

STR: Go to Preferences > Home and look for the drop down input to set the number of rows for Top Sites. The field is truncated and reads "2 ro..." when it should read "2 rows". [see attached image]

	Kate Hudson :k88hudson
	Comment 1 • 6 years ago

This bug seems to only affect certain people on a Macbook pro, but there seems to be no correlation with OS version / machine that I can find...

:jaws, is this like anything you have seen before / have any ideas about how to reproduce or fix it?

	Jared Wein [:jaws] (please needinfo? me)
	Comment 2 • 6 years ago

There is two ways to fix this.

First, the issue is happening because the <label> has flex="1" but the menulist doesn't. There are oddities around the horizontal sizing of the menulist when it is next to a flex="1" item. To fix this, you could do like you have done above with the "Homepage and new windows" row and surround this menulist with a vbox that has flex="1". I have confirmed locally by changing [1] that this will fix the bug.

Secondly, we shouldn't be putting menulists at the end of a row in preferences. This goes against a previous design decision and introduced a visual inconsistency within the preferences. See the "Privacy & Security" section's "History" and "Cookies and Site Data" sections. The menulist should be inline with the text of the label, so removing the flex="1" on the <label> is the better way to fix this.

Lastly, since there are only two options in the menulist we should consider using radio buttons as they work better with smaller number of options.

[1] https://searchfox.org/mozilla-central/rev/c0d81882c7941c4ff13a50603e37095cdab0d1ea/browser/extensions/activity-stream/lib/AboutPreferences.jsm#229

	Ed Lee :Mardak Assignee
	Comment 3 • 6 years ago

> To fix this, you could do like you have done above with the "Homepage and new windows" row and surround this menulist with a vbox that has flex="1".
I don't think it's desired to have the menu flex.

> The menulist should be inline with the text of the label
The menulist inline with its related text isn't quite applicable here as the rows are just their own control that happen to share the line with the description. I could see the inline pattern make more sense if it was something like "Show [2 ▼] rows."

> since there are only two options in the menulist we should consider using radio buttons
This control is intended to be for multiple sections and offering up to 4 rows (or more?) but the backend currently only happens to support 2 rows of top sites and a fixed number for highlights.

	Ed Lee :Mardak Assignee
	Comment 5 • 6 years ago

Looks like others have run into this before, e.g., bug 549697 comment 73

	[github robot]
	Comment 6 • 6 years ago

Commit pushed to master at https://github.com/mozilla/activity-stream

https://github.com/mozilla/activity-stream/commit/0496759e168f7d782329b375269feb5aa071d501
fix(preferences): Prevent "2 rows" menu item from getting cropped (#4168)

Fix Bug 1457239 - Setting for number of rows is truncated making the label not readable unless you click on it

	Ed Lee :Mardak Assignee
	Comment 7 • 6 years ago

https://hg.mozilla.org/mozilla-central/rev/609a288fe314

	Ryan VanderMeulen [:RyanVM]
	Comment 8 • 6 years ago

If you want to create a backport patch for just the upstream PR, I'd entertain taking it in 61. Otherwise, let's let this ride 62.

	Ciprian Muresan [:cmuresan], Ecosystem QA
	Comment 9 • 6 years ago

I wasn't able to reproduce this issue on Windows 10 x64, Windows 7 x64, Mac 10.13.3 on a MacBook Pro early 2015, Mac 10.13.4 and 10.12.6 on an iMac late 2013, Ubuntu 16.04 x64 or Arch Linux x64 on builds that should have been broken. In my case, the "2 rows" string was always fully displayed.
@aaron, could you please see if the issue is still reproducible for you on the latest Nightly?

	Aaron Benson Reporter
	Comment 10 • 6 years ago

I can't reproduce either ... looks great now!

	Ryan VanderMeulen [:RyanVM]
	Comment 11 • 6 years ago

Sounds like we can just let this ride the trains then.

	Cristian Baica [:cbaica], Release Desktop QA
	Comment 12 • 6 years ago

I have managed to reproduce this issue only on Macbook Pro using Firefox 61.0a1 buildID: 20180426100055.

The issue is verified as fixed using Firefox 62.0b9 buildID: 20180713213322 and Firefox 63.0a1 buildID: 20180717220130 on Macbook Pro. I have also checked the mentioned Fx versions on Windows 10 x64 and Ubuntu 16.04 x64 but the issue was not occurring on those environments in the first place.