Closed Bug 1562894 Opened 1 year ago Closed 1 year ago

Top sites PIN option is always highlighted

Categories

(Firefox :: New Tab Page, defect, P1)

Desktop
All
defect

Tracking

()

VERIFIED FIXED
Firefox 70
Iteration:
70.1 - Jul 8 - 21
Tracking Status
firefox-esr60 --- unaffected
firefox-esr68 --- unaffected
firefox67 --- unaffected
firefox68 --- unaffected
firefox69 --- verified
firefox70 --- verified

People

(Reporter: pablo.muir, Assigned: emcminn)

References

(Regression)

Details

(Keywords: github-merged, regression)

Attachments

(3 files)

[Description:]
The PIN option is always highlighted in the Top sites area.

[Environment:]
69.0a1 Nightly Win10 / Ubuntu 18.04 / Macos 10.14.4 (all affected)
68.0b14 Beta Win10 no/ Ubuntu 18.04 no/ Macos 10.14.4 (not affected)
67.0.4 Release Win10 no/ Ubuntu 18.04 no/ Macos 10.14.4 (not affected)

[Steps:]
Launch firefox
Open new tab
Check Top Sites
Click on "..." on one of the top sites
See "Pin" option

[Actual Result:]
Pin option is always highlighted, even if you hover over another option

[Expected Result:]
Pin option should not be highlighted, it should be highlighted only if you hover over it.

[Notes:]
Mozregression:
changeset: c04923179a923f5a94af9b6df475c266adbf39f0
pushlog_url: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=c04923179a923f5a94af9b6df475c266adbf39f0&tochange=a72f0a08f652ab309328230659c97660e13cfd27

Keywords: regression

Note this issue not only affects Top Sites, it also happens on the "Recommended by Pocket" section. As the "bookmark" option is also always selected once you click on "..."

Reverting bug 1559383 seems to fix the problem locally for me. It's just that opening the menu focuses the item, so pointing with the mouse also shows a hover effect too in addition to the focus.

Flags: needinfo?(emcminn)
Regressed by: 1559383

To me it looks like the issue is that if the user switches keyboard and mouse, the keyboard highlight remains when the mouse highlights another option. Should be able to fix this by allowing only one option to be focused at a time. I'll take a look!

Flags: needinfo?(emcminn)
Assignee: nobody → emcminn

So we've identified a way to fix this but it'll take a pretty hefty refactor - for now I'll remove the focus behavior entirely, and fix both that and the highlighting with a future patch.

Blocks: 1565293
Status: NEW → RESOLVED
Iteration: --- → 70.1 - Jul 8 - 21
Closed: 1 year ago
Keywords: github-merged
Priority: -- → P1
Resolution: --- → FIXED
Target Milestone: --- → Firefox 70

I have verified that the issue is no longer reproducible on the latest Nightly 70.0a1 (Build ID 20190713215744) on Windows 10, macOS 10.14, and Arch Linux 14.4.3.

@Ed, given that bug 1559383 has been reverted, should we reopen it in order to track the issue moving forward?

Status: RESOLVED → VERIFIED
Flags: needinfo?(edilee)

Sounds reasonable. There's already a PR to fix things https://github.com/mozilla/activity-stream/pull/5161 I'll update it to point to the reopened bug

Flags: needinfo?(edilee)

Is this something we should consider for Beta uplift for Fx69?

Flags: needinfo?(emcminn)

(In reply to Ryan VanderMeulen [:RyanVM] from comment #10)

Is this something we should consider for Beta uplift for Fx69?

I think so!

Flags: needinfo?(emcminn)

OK, just waiting on a patch and approval request :)

Comment on attachment 9081306 [details]
Bug 1562894 - Top sites PIN option is always highlighted

Beta/Release Uplift Approval Request

  • User impact if declined: Focus behavior of context menus will be incorrect.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: See steps in https://bugzilla.mozilla.org/show_bug.cgi?id=1562894
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): This patch backs out a change to focus behavior and reverts it to the previous correct behavior.
  • String changes made/needed: none
Attachment #9081306 - Flags: approval-mozilla-beta?
Flags: qe-verify+

Comment on attachment 9081306 [details]
Bug 1562894 - Top sites PIN option is always highlighted

Thanks for the approval request. Approved for 69.0b11.

Attachment #9081306 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Ryan, I grafted the patch here and another bug got in, bug 1565293: https://irccloud.mozilla.com/file/ABNAAZp4/image.png

I backed that bug out, and landed this one. Sorry for the mess.

Flags: needinfo?(ryanvm)
QA Whiteboard: [qa-triaged]
Flags: needinfo?(ryanvm)

I have verified that this issue is no longer reproducible with the latest Firefox Beta (69.0b14 Build ID - 20190815163925) installed, on Windows 10 x64, Arch Linux and Mac 10.14.5. Now the first option from the menu is no longer highlighted by default.

Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.