Closed Bug 1457239 Opened 4 years ago Closed 4 years ago
Setting for number of rows is truncated making the label not readable unless you click on it
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]
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?
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  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.  https://searchfox.org/mozilla-central/rev/c0d81882c7941c4ff13a50603e37095cdab0d1ea/browser/extensions/activity-stream/lib/AboutPreferences.jsm#229
> 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.
Assignee: nobody → edilee
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
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
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.
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?
I can't reproduce either ... looks great now!
4 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.
Component: Activity Streams: Newtab → New Tab Page
You need to log in before you can comment on or make changes to this bug.