1570663 - Regression? Favicons in Drop-Down have a wrong position after clicking on the arrow button and typing an address

Status: VERIFIED FIXED

(Firefox :: Address Bar, defect, P1)

Description

[Mehmet]

Attachment: Wrong_Icon_Position_in_Drop_Down.mov

Nightly 70.0a1 (2019-07-31) (64-Bit)
macOS 10.14.5

STR:
1.) Visit google.com
2.) Click on the Drop Down Arrow, so that the Drop Down appears
3.) Now type an address into the Address Bar

Actual: Wrong position of the Favicons and Text.

Expected: Favicons and Text should be left aligned with the Glass icon.

A screencast is attached.

Thanks.
Mehmet

Comment 1

[Marco Bonardo [:mak]]

May this be related to the new protection shield.

Comment 2

[Tim Huang[:timhuang]]

I ran a bisect and it shows that Bug 1562881 cause this regression.

I believe the root cause here is that the awesome bar aligns to the identity box, which is at the second position, in the case where the awesome bar dropdown is triggered by clicking the down arrow button. And the awesome bar will be reused if keeping typing in the address bar, so the favicon and text won't align to the glass icon anymore because right now the glass icon is at the first position but the awesome bar still aligns to the old position, which is the second position.

I think the awesome bar should align to the shield icon in the case that awesome bar is triggered by the down arrow button and the pageproxystate is valid.

What do you think, adw?

Comment 3

[Drew Willcoxon :adw]

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

I think the awesome bar should align to the shield icon in the case that awesome bar is triggered by the down arrow button and the pageproxystate is valid.

The important things imo, in order:

1. The text in the popup should always align with the text in the input
2. There shouldn't be a giant gap between the icons and text in the popup

If the popup icons align with the shield icon, then either the popup text won't align with the input text, or it will but there will be a big gap between the icons and text. So if that's what you mean, I don't agree.

We should keep alignments as they are now: When the page proxy state is valid, popup icons align with the lock and text is aligned, and when it's invalid, popup icons align with the glass icon and the text is aligned. The only thing in this bug that needs fixing is the case where the popup is opened when the proxy state is valid and it remains open when the state becomes invalid.

But we should probably get Verdi's opinion, so I'll set a needinfo.

Currently we set the padding in the popup only when the popup is opened. Now the padding needs to change when going from a valid proxy state to invalid. https://searchfox.org/mozilla-central/rev/e0b0c38ee83f99d3cf868bad525ace4a395039f1/browser/components/urlbar/UrlbarView.jsm#388

I or someone else on the quantumbar team could actually take this bug, if you want. Right now I'm more concerned about bug 1570251. The problem there still seems to be what I said in bug 1570008 comment 21.

Comment 4

[Drew Willcoxon :adw]

I looked at what we did before the shield icon, when we still showed the "i" icon and lock together. For a valid page proxy state, we did as you suggested: align popup icons with the "i" icon, and don't align the text. So I guess that's what we should continue to do after all.

Comment 5

[Tim Huang[:timhuang]]

Attachment: Bug 1570663 - Align favicons in the awesome bar with the shield icon if the pageproxystate is valid. r?adw!

This patch changes the alignd icon of the awesome bar to the shield icon
if the pageproxystate is valid.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/790e7615ee18
Align favicons in the awesome bar with the shield icon if the pageproxystate is valid. r=adw

Comment 7

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/790e7615ee18

Comment 8

[Verdi [:verdi] Best to slack me]

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

I looked at what we did before the shield icon, when we still showed the "i" icon and lock together. For a valid page proxy state, we did as you suggested: align popup icons with the "i" icon, and don't align the text. So I guess that's what we should continue to do after all.

Yes! Thanks everyone.

Comment 9

[Negritas Sergiu, Desktop QA]

I managed to reproduce the issue using Fx nightly 70.0a1 (2019-07-31), I can confirm that the issue is fixed on the latest beta and nightly.