1259132 - The new device-list label in RDM isn't properly aligned vertically

Status: VERIFIED FIXED

(DevTools :: Responsive Design Mode, defect, P1)

Description

[Patrick Brosset <:pbro>]

Attachment: device-list-alignment.png

See screenshot.
The string that appears in the selected option of the "device list" drop-down isn't vertically aligned properly (tested with a build of fx-team on windows 10)

Comment 1

[Marco Mucci [:MarcoM]]

Hi Ryan, are we going to bring in bug 1259132 and remove bug 1240912 or 1240907?  Or is one of the two current release bugs better for Gabriel?

Comment 2

[Marco Mucci [:MarcoM]]

Discussed at team standup - bug added to Release 49.

Comment 3

[Jaideep Bhoosreddy [:jbhoosreddy]]

Hello Patrick. Can I get a little more clarification on this bug?

Comment 4

[Jaideep Bhoosreddy [:jbhoosreddy]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=35c36dbd933f

Comment 5

[Jaideep Bhoosreddy [:jbhoosreddy]]

Attachment: 1259132.patch

So, in this patch, I have increased the height of the viewport toolbar by 2px, which seems to make it work. I think that this issue is caused by the Tohoma font, and I could not find a way to prevent it from happening without changing the height.

I also change the position of device selectors from pixels to percentage. The icon seemed "very" slightly vertically misaligned to the top and I tried to fix that.

Comment 6

[Jaideep Bhoosreddy [:jbhoosreddy]]

Attachment: 1259132.patch [1.0]

Update from previous bug patch: modified the viewport rotate button.

Comment 7

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8760507 [details] [diff] [review]
1259132.patch [1.0]

Review of attachment 8760507 [details] [diff] [review]:
-----------------------------------------------------------------

:gl, can you review this one?

Comment 8

[Helen V. Holmes (:helenvholmes) (:✨)]

Comment on attachment 8760507 [details] [diff] [review]
1259132.patch [1.0]

This is still looking a smidge too low on my end: http://cl.ly/2i182Y0I0j22

Comment 9

[Jaideep Bhoosreddy [:jbhoosreddy]]

Attachment: 1259132.patch [2.0]

Comment 10

[Jaideep Bhoosreddy [:jbhoosreddy]]

Comment on attachment 8763637 [details] [diff] [review]
1259132.patch [2.0]

I looked into the computed values for OSX and Windows and found what was causing the issue. Checked it with uppercase transformation, to make sure it was properly aligned. The selector icon is also not aligned but I am working on it in another bug, so I will make that change there.

Comment 11

[Helen V. Holmes (:helenvholmes) (:✨)]

Comment on attachment 8763637 [details] [diff] [review]
1259132.patch [2.0]

Review of attachment 8763637 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me!

Comment 12

[Carsten Book [:Tomcat]]

has problems to apply:

applying 1259132.patch
patching file devtools/client/responsive.html/index.css
Hunk #1 FAILED at 173
1 out of 1 hunks FAILED -- saving rejects to file devtools/client/responsive.html/index.css.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working directory
errors during apply, please fix and qrefresh 1259132.patch

Comment 13

[Jaideep Bhoosreddy [:jbhoosreddy]]

Attachment: 1259132.patch [3.0]

Re-upload patch to fix merge problems.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/fx-team/rev/9560c3d5e225
vertically aligning device-list label in rdm which is not aligned on windows; r=helenvholmes

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/9560c3d5e225

Comment 16

[Mihai Boldan, Desktop QA [:mboldan]]

Attachment: horizonatlly aligned.png

I managed to reproduce this issue on Firefox 50.0a1 (2016-06-16) and on Windows 10 x64.
I confirm that the new device-list label correctly aligned vertically.
While testing, I noticed that the label is not properly aligned horizontally(see the attached screenshot).
Note that the issue described above is reproducible on all the tested platforms.
Should I log a new bug for this issue, or it's ok to reopen this one?

The tests were performed on Latest Nightly 50.0a1 (2016-07-03) and on Windows 10 x64, Mac OS X 10.11.1 and on Ubuntu 14.04 x86.

Comment 17

[Patrick Brosset <:pbro>]

(In reply to Mihai Boldan, QA [:mboldan] from comment #16)
> While testing, I noticed that the label is not properly aligned
> horizontally(see the attached screenshot).
> Note that the issue described above is reproducible on all the tested
> platforms.
> Should I log a new bug for this issue, or it's ok to reopen this one?
This is a separate issue, I think a new bug needs to be filed. Looking at the patch that landed here, I don't think it is at the source of this change though. Maybe the CSS for this part of the layout has changed in another bug.

Comment 18

[Jaideep Bhoosreddy [:jbhoosreddy]]

(In reply to Mihai Boldan, QA [:mboldan] from comment #16)
> Created attachment 8767662 [details]
> horizonatlly aligned.png
> 
> I managed to reproduce this issue on Firefox 50.0a1 (2016-06-16) and on
> Windows 10 x64.
> I confirm that the new device-list label correctly aligned vertically.
> While testing, I noticed that the label is not properly aligned
> horizontally(see the attached screenshot).
> Note that the issue described above is reproducible on all the tested
> platforms.
> Should I log a new bug for this issue, or it's ok to reopen this one?
> 
> The tests were performed on Latest Nightly 50.0a1 (2016-07-03) and on
> Windows 10 x64, Mac OS X 10.11.1 and on Ubuntu 14.04 x86.

This change is due to Bug 1254390. To enable flexible sizing of the device selector.

Comment 19

[Mihai Boldan, Desktop QA [:mboldan]]

I logged Bug 1284463 for the issue described above. 
I am marking this issue Verified Fixed, since is no longer reproducible.

The tests were performed on Firefox 50.0a1 (2016-07-04) and on Windows 10 x64, Mac OS X 10.11.1 and on Ubuntu 14.04 x86.