Closed Bug 1570008 Opened 2 years ago Closed 2 years ago

Regression: Address Bar alignments are broken

Categories

(Firefox :: Address Bar, defect, P1)

defect

Tracking

()

RESOLVED FIXED
Firefox 70
Tracking Status
firefox-esr60 --- unaffected
firefox-esr68 --- unaffected
firefox68 --- unaffected
firefox69 --- unaffected
firefox70 --- fixed

People

(Reporter: mehmet.sahin, Assigned: timhuang)

References

(Blocks 1 open bug, Regression)

Details

(Keywords: regression, Whiteboard: [privacy-panel][skyline])

Attachments

(9 files)

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.

Attached image Point_1_actual.png
Attached image Point_1_expected.png
Attached image Point_2_actual.png
Attached image Point_2_expected.png
Priority: -- → P1

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.

Flags: needinfo?(mak77)

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.

Flags: needinfo?(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.

Flags: needinfo?(nhnt11) → needinfo?(tihuang)
Flags: needinfo?(mak77)

(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.

Flags: needinfo?(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.

Flags: needinfo?(adw)

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.

(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?

Flags: needinfo?(tihuang) → needinfo?(adw)
Assignee: nobody → tihuang

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.

Whiteboard: [privacy-panel][skyline]
Status: NEW → ASSIGNED
Duplicate of this bug: 1570251
Attachment #9081798 - Attachment description: Bug 1570008 - Remove the 'margin-inline-start' for the magnifying glass icon. r?nhnt11! → Bug 1570008 - Remove the 'margin-inline-start' and change 'margin-inline-end' for the magnifying glass icon. r?nhnt11!
Pushed by tihuang@mozilla.com:
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

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?

Flags: needinfo?(adw) → needinfo?(tihuang)

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

Keywords: leave-open

(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.

Flags: needinfo?(tihuang) → needinfo?(nhnt11)

(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.

Attached image 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 :)

Attached image actual.png
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Blocks: 1570251

(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...

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

Target Milestone: --- → Firefox 70
Flags: needinfo?(nhnt11)
You need to log in before you can comment on or make changes to this bug.