1570008 - Regression: Address Bar alignments are broken

Status: RESOLVED FIXED

(Firefox :: Address Bar, defect, P1)

Description

[Mehmet]

70.0a1 (2019-07-29) (64-Bit)
OS=All Desktop

1.) Compare Address Bar Text with the Drop Down Text position.

-> No longer aligned.

2.) Compare Protection Shield Position with the Magnifiying Glass Position in the Address Bar and with the icons in the Dropdown.

-> No longer aligned.

This is probably a regression from bug 1569182.

Comment 1

[Mehmet]

Attachment: Point_1_actual.png

Comment 2

[Mehmet]

Attachment: Point_1_expected.png

Comment 3

[Mehmet]

Attachment: Point_2_actual.png

Comment 4

[Mehmet]

Attachment: Point_2_expected.png

Comment 5

[Mehmet]

Attachment: Point_2_with_Dropdown_actual.png

Comment 6

[Mehmet]

Attachment: Point_2_with_Drop_Down_expected.png

Comment 7

[(inactive) Nihanth Subramanya [:nhnt11]]

This is caused by recent work done on the behavior and styling of the shield and identity icons in the URL bar. Marco, do you think someone who has more experience with the Awesomebar dropdown code might be able to help here to adjust the alignments of the icons in the dropdown? Otherwise, I or :timhuang can take it.

Comment 8

[Drew Willcoxon :adw]

Nihanth, my question is, it looks like there's now more spacing in the input between the text and the icons. Is that intentional? We actually recently shrunk it by 1pt, before this (bug 1548198). We need to either shrink that space back down, or increase the space in the urlbar popup to match.

Comment 9

[(inactive) Nihanth Subramanya [:nhnt11]]

Ah, I see now, that's interesting. The change in the spacing of the magnifying glass icon isn't intentional AFAIK. Tim, you updated this in bug 1567185. Could you take a look please? Seems like there are two action items in this bug:

1. Ensure the shield icon and the magnifying glass icon take the same space/alignment in the URL bar.
2. Ensure that the awesomebar dropdown items align with the magnifying glass icon.

If you don't have cycles to take this feel free to comment to that effect and maybe I can pick it up.

Comment 10

[(inactive) Nihanth Subramanya [:nhnt11]]

(In reply to Nihanth Subramanya [:nhnt11] from comment #9)

Ah, I see now, that's interesting. The change in the spacing of the magnifying glass icon isn't intentional AFAIK. Tim, you updated this in bug 1567185. Could you take a look please? Seems like there are two action items in this bug:

1. Ensure the shield icon and the magnifying glass icon take the same space/alignment in the URL bar.
2. Ensure that the awesomebar dropdown items align with the magnifying glass icon.

If you don't have cycles to take this feel free to comment to that effect and maybe I can pick it up.

Drew, maybe you can help with the second item if you have time? Might be more efficient if someone more familiar with that code/styles takes that part.

Comment 11

[Drew Willcoxon :adw]

Basically I'm just wondering whether the spacing in the input is as designed. If so, then we should adjust only the spacing in the popup to match.

Previously, the spacing between the left edge of the input and the shield, between the shield and lock, and between the lock and text was all the same, more or less. Now, there's obviously more spacing between the lock and text than between the other elements. That accounts for why the popup text is no longer aligned afaict. (The same goes for when the proxy state is invalid and the magnifying glass is shown instead of the lock and shield.)

I can make whatever adjustments to the popup that we end up deciding we need.

Comment 12

[Mehmet]

Would be nice, if the Glass in the Adressbar could also be aligned with the Shield as it was before. Now, when you start typing, there is a position-flicker when changing between Shield and Glass.

Comment 13

[Tim Huang[:timhuang]]

(In reply to Nihanth Subramanya [:nhnt11] from comment #9)

Ah, I see now, that's interesting. The change in the spacing of the magnifying glass icon isn't intentional AFAIK. Tim, you updated this in bug 1567185. Could you take a look please? Seems like there are two action items in this bug:

1. Ensure the shield icon and the magnifying glass icon take the same space/alignment in the URL bar.
2. Ensure that the awesomebar dropdown items align with the magnifying glass icon.

If you don't have cycles to take this feel free to comment to that effect and maybe I can pick it up.

The margin that I introduced is to put the magnifying glass icon to the right place. However, I believe this is out-dated since the margin has been changed in bug 1569182. Right now, I think we don't need the margin anymore. So, the shield icon and the magnifying glass icon would have the same layout:
|- 6px padding-|- 16px icon width-|- 6px padding -|

I can write the patch for the magnifying glass icon, but does this layout matches to awesomebar?

Comment 14

[Marco Bonardo [:mak]]

the relevant code should be here https://searchfox.org/mozilla-central/rev/38c88cbf4be87dfa0636d15a5d3599a8ea0d1a72/browser/components/urlbar/UrlbarView.jsm#375

Comment 15

[Tim Huang[:timhuang]]

Attachment: Bug 1570008 - Remove the 'margin-inline-start' and change 'margin-inline-end' for the magnifying glass icon. r?nhnt11!

We don't need this margin since the layout of the identity icon has been
updated. Right now, the shield icon and identity icon have the same
layout. So, even the shield icon is hidden, the identity icon can
perfectly align to the position of the shield icon.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/81efb80c315a
Remove the 'margin-inline-start' and change 'margin-inline-end' for the magnifying glass icon. r=nhnt11

Comment 18

[Drew Willcoxon :adw]

This fixed the alignment when the page proxy state is invalid, but it's still off when the proxy state is valid, by 2 points it looks like. (You can test that by visiting a page and then clicking the dropdown arrow in the urlbar.)

Also, and it may be related, the space between the lock icon (when the proxy state is valid) and the input text is ~12.5 points (on my 2x screen), but the space between the glass icon (when the proxy state is invalid) and the input text is only ~8.5 points. Shouldn't they be the same, more or less?

Comment 19

[(inactive) Nihanth Subramanya [:nhnt11]]

I feel like this bug may need another patch or two to fully "fix", so marking this leave-open for now.

Comment 20

[Tim Huang[:timhuang]]

(In reply to Drew Willcoxon :adw from comment #18)

This fixed the alignment when the page proxy state is invalid, but it's still off when the proxy state is valid, by 2 points it looks like. (You can test that by visiting a page and then clicking the dropdown arrow in the urlbar.)

I believe the reason is that the tracking protection icon has a 2px space between the edge of the SVG and the shield. So, if we center-aligned the tracking protection icon and the magnifying glass icon, there will be a 2px difference here. Nihanth and I talked about this and we decided to center-aligned them instead of left-aligned. Personally, I think this is a UX problem.

Nihanth, what's your suggestion here?

Also, and it may be related, the space between the lock icon (when the proxy state is valid) and the input text is ~12.5 points (on my 2x screen), but the space between the glass icon (when the proxy state is invalid) and the input text is only ~8.5 points. Shouldn't they be the same, more or less?

We intentionally make the space between glass icon and url smaller in order to balance the spaces around the glass icon. That's why they are different.

Comment 21

[Drew Willcoxon :adw]

(In reply to Tim Huang[:timhuang] from comment #20)

We intentionally make the space between glass icon and url smaller in order to balance the spaces around the glass icon. That's why they are different.

But that means that in the popup, the space between the icons and text has to also be different, depending on whether the page proxy state is valid or not. That doesn't seem right to me.

Comment 22

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/81efb80c315a

Comment 23

[Mehmet]

Attachment: expected.png

Thanks for the initial fix of the bug. I tested latest Nightly and as Drew mentioned in comment 18 and 21, it looks like the Text input needs to be shifted 2px to the left when the proxy state is valid. That would look much more balanced with the other spacings in the Address Bar and be aligned again with the Drop Down text. Thanks :)

Comment 24

[Mehmet]

Attachment: actual.png

Comment 25

[Dão Gottwald [:dao]]

(In reply to Nihanth Subramanya [:nhnt11] from comment #19)

I feel like this bug may need another patch or two to fully "fix", so marking this leave-open for now.

Oops, missed this. This makes it harder to track patches and I'd suggest filing separate bugs instead. I also just reopened bug 1570251...

Comment 26

[Pascal Chevrel:pascalc]

Marking 70 as fixed since the remaining work will be done in bug 1570251.