1709924 - Mouseover/hover and click problems with row labels

Status: VERIFIED FIXED

(Firefox :: Address Bar, defect, P1)

Description

[Drew Willcoxon :adw]

I noticed some problems with mouseover and clicking related to the row labels introduced in bug 1708621:

• If you keyboard-select a row with a label and then mouseover it, it gets the hover styling. It should keep the selected styling
• Mousing over a label shows the action text in the row
• Clicking a label clicks the row

Comment 1

[Drew Willcoxon :adw]

[Tracking Requested - why for this release]: This is a defect in bug 1708621, which is tracking 89 and needs to be uplifted.

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

Since the status are different for nightly and release, what's the status for beta?
For more information, please visit auto_nag documentation.

Comment 3

[Drew Willcoxon :adw]

Attachment: Bug 1709924 - Fix mouseover/hover and click problems with row labels.

We need to modify or augment basically every :hover rule to account for the
fact that the ::before pseudo-element for the label is a child of the row.

It makes the CSS harder to read unfortunately. I don't see a way around that
without changing how labels are implemented. We could go back to my original
approach of creating separate "decoration" results for them, or it might help to
modify the DOM, for example so that there's a urlbarView-row-outer between
urlbarView-row and urlbarView-row-inner, and then urlbarView-row-outer
would be the element that gets the hover styling instead of the row itself.

We need to uplift a fix for this bug to 89 though, so making larger changes like
that isn't wise right now.

I've tested this with various types of results using the test patch I'll post in
the bug. I tested with the default, light, and dark themes.

Comment 4

[Drew Willcoxon :adw]

Attachment: label-test.diff

I've been using this to check a variety of result types with the main patch. It adds browser_labelTest.js, which creates a provider that returns a bunch of results.

Comment 5

[Drew Willcoxon :adw]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=744aaa959a6476809e33d3f2a063bd99130024f6

Comment 6

[Marco Bonardo [:mak]]

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

• If you keyboard-select a row with a label and then mouseover it, it gets the hover styling. It should keep the selected styling

Can we leave this bug unfixed for 89 and handle it apart? it's my understanding that would allow us to avoid invalidating a bunch of QA on the Proton urlbar.

• Mousing over a label shows the action text in the row

is this actually replacing action text with url or just showing the hover effect on the action text?
I'd also leave this unfixed in 89 for the same reason

• Clicking a label clicks the row

This looks like the only "serious" bug that imo should be fixed and uplifted.

I'm trying to avoid landing things in 89 that may affect all the Proton validation we made.

Comment 7

[Drew Willcoxon :adw]

The approach you suggested on Slack is much better and I've updated the patch. That should take care of all these concerns. Thank you!

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

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5575c043463d
Fix mouseover/hover and click problems with row labels. r=mak

Comment 9

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/5575c043463d

Comment 10

[Drew Willcoxon :adw]

Comment on attachment 9220918 [details]
Bug 1709924 - Fix mouseover/hover and click problems with row labels.

Beta/Release Uplift Approval Request

• User impact if declined: We need this for the Firefox Suggest/Quick Suggest rollout in 89.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: Please see comment 0
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This only affects rows in the urlbar panel that have labels, which is the feature that will be rolled out. Mostly this is only cosmetic styling changes. It also affects clicks in the urlbar panel, but only for rows that have labels, and that part comes with a test.
• String changes made/needed:

Comment 11

[Pascal Chevrel:pascalc]

Comment on attachment 9220918 [details]
Bug 1709924 - Fix mouseover/hover and click problems with row labels.

Approved for 89 beta 12, thanks.

Comment 12

[Pascal Chevrel:pascalc]

https://hg.mozilla.org/releases/mozilla-beta/rev/1039cf4069d6

Comment 13

[Cosmin Muntean [:cmuntean], Ecosystem QA]

I have verified this bug on the latest Nightly 90.0a1 build (Build ID: 20210510212929) on Windows 10 x64, macOS 10.15.7 and Ubuntu 20.04 x64.

• The issues mentioned in comment 0 are no longer reproducible with the latest Nightly builds.

I will verify the issue also on Firefox Beta 89.0b12 as soon as this version is available.

Comment 14

[Cosmin Muntean [:cmuntean], Ecosystem QA]

I have verified this bug on the latest Beta 89.0b12 build (Build ID: 20210513185752) on Windows 10 x64, macOS 10.15.7 and Ubuntu 20.04 x64.

• The issues mentioned in comment 0 are no longer reproducible with Beta 89.0b12.