1399138 - Dimensions ­-PaddingSpacingMargins doesn't match the specifications (Compact mode)

Status: NEW

(Firefox :: Theme, defect, P5)

Description

[Valentina Claudia Ona]

Attachment: Compact Dimensions PaddingSpacingMargins.bmp

Mozilla/5.0 (Windows NT 6.3; WOW64; rv:57.0) Gecko/20100101 Firefox/57.0
Build ID:  20170910220126

[Affected versions]:  Nightly 57.0a1

[Affected platforms]:  Windows 8.1 x64

[Steps to reproduce]:
1.Launch Nightly 57.0a1 with a new profile.
2.Go to Customization menu
3.Select Compact from the Density.

[Expected result]:
The dimensions should match the specifications.

[Actual result]:
The dimensions doesn't match as specifications.
Please verify the attachment and what is not correct is marked with yellow.

Comment 1

[Valentina Claudia Ona]

I verify this issue on Windows 10 x64 and it's also reproducible.

Comment 2

[Brindusa Tot, Desktop QA]

Attachment: Windows10_CompactDensity_IdentityBlockDimensions.jpg

For Compact mode, on Windows 10, the distance between buttons from URL bar is 4px instead of 2px.

Also, on Windows 10, for the identity block, few dimensions do not respect the Photon specs - please see attachment(Windows10_CompactDensity_IdentityBlockDimensions.jpg)

Comment 4

[roxleitan]

Attachment: Compact dimension Ubuntu 16.04.png

Also, on Ubuntu 16.04, some distances between buttons do not respect the Photon specs - please see attachment

Comment 5

[Dão Gottwald [:dao]]

I think bug 1398103 fixed this, or most of it. Valentina or Brindusa or Roxana, could you please check with the latest nightly?

Comment 6

[Valentina Claudia Ona]

Attachment: -Compact Dimensions PaddingSpacingMargins.bmp

I verified this issue using Nightly 58.0a1 with Build ID 20171102222620 on Windows 10 x64, Windows 7 x32, Windows 8.1 and Ubuntu 16.04 but the issue is not resolved.
Please see the attachment.

Comment 7

[roxleitan]

Attachment: Compact dimension Ubuntu 16.04 Nightly 58.0a1.png

Comment 8

[Dale Harvey (:daleharvey)]

Attachment: Bug 1399138 - Fix identity block spacing in compact mode.

Review commit: https://reviewboard.mozilla.org/r/202020/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/202020/

Comment 9

[Dale Harvey (:daleharvey)]

The margin+padding around the spacer is wrong in compact mode here, I figured css vars were cleaner than adding :root:not([uidensity=compact]) before every selector that gave this extra margin but happy to switch

Comment 10

[Dale Harvey (:daleharvey)]

Attachment: Strange margin

The space between the (i) and the connection icon is behaving strangely for me, it is given a 4px margin (https://searchfox.org/mozilla-central/source/browser/themes/shared/identity-block/identity-block.inc.css#79) however on screen its measuring at 7px, if I reduce that to 0 then the icons actually overlap, incrementally it looks like the margin is doubling but its not a dpi issue and looks extreme for a rendering glitch

Comment 11

[Dale Harvey (:daleharvey)]

actually relooking at ^, it looks like tracking protection icon is interfering and having an effect on layout when its not actually displayed

Comment 12

[Dale Harvey (:daleharvey)]

Changing priority so it orders correctly since its assigned

Comment 13

[Dão Gottwald [:dao]]

(In reply to Dale Harvey (:daleharvey) from comment #11)
> actually relooking at ^, it looks like tracking protection icon is
> interfering and having an effect on layout when its not actually displayed

The icon is hidden using a negative margin so that it can slide into view when needed: https://searchfox.org/mozilla-central/rev/c8e791091973825680bbba807fc1c4f5bda0f5a1/browser/themes/shared/identity-block/identity-block.inc.css#167-177

(In reply to Dale Harvey (:daleharvey) from comment #9)
> The margin+padding around the spacer is wrong in compact mode here

TBH I'm not sure that adjusting this for compact mode is worthwhile.

Comment 14

[Dão Gottwald [:dao]]

Comment on attachment 8930908 [details]
Bug 1399138 - Fix identity block spacing in compact mode.

https://reviewboard.mozilla.org/r/202020/#review213796

Comment 15

[Dale Harvey (:daleharvey)]

Unassigning for parental leave