1668247 - The elements from the "Saved to Pocket" doorhanger are not visibly focused through keyboard navigation

Status: VERIFIED FIXED

(Firefox :: Pocket, defect, P1)

Description

[Marius Coman [:mcoman], Ecosystem QA]

Attachment: rec of the issue 3.gif

[Affected versions]:

• Firefox Release 81.0 - Build ID: 20200917005511
• Firefox Beta 82.0b5 - Build ID: 20200929175845
• Firefox Nightly 83.0a1 - Build ID: 20200929213959

[Affected Platforms]:

• Windows 10 x64
• macOS 10.15
• Ubuntu 18.04 x64

[Prerequisites]:

• You are logged in to Pocket.

[Steps to reproduce]:

1. Open the browser with the profile from prerequisites.
2. Click the "Save to Pocket" button from the right part of the "Address Bar".
3. Press the "Tab" key twice and observe the behavior.

[Expected result]:

• The "Remove Page" hyperlink is visibly focused.

[Actual result]:

• No elements from the Pocket doorhanger are focused.

[Notes]:

• The functionality part of the elements is not affected. For example, if after step 3 the "Enter" key is pressed, the website will be removed from Pocket.
• Attached a screenshot of the issue.

Comment 1

[Scott [:thecount] Downe]

Attachment: Bug 1668247 - fixing some jank with keyboard nav on save to pocket button doorhanger

Comment 2

[Scott [:thecount] Downe]

My fix makes this work for everything but premium tags. I couldn't get it working easily within the scope I gave myself.

Premium tags navigation always had this issue though, even before on save recs, so I don't think it should block.

It fixes it to the point where the tabbing works for the new elements added in on save recs, and for links that were easy enough to do.

Premium tab navigation can happen in bug 1668662

Comment 3

[Scott [:thecount] Downe]

Comment on attachment 9179095 [details]
Bug 1668247 - fixing some jank with keyboard nav on save to pocket button doorhanger

Beta/Release Uplift Approval Request

• User impact if declined: UI/UX improvements to a feature we are looking to pref on.
• 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: 1. Log into Pocket

2. Click the "Save to Pocket" button from the right part of the "Address Bar". You can do this with keyboard nav too, should work.
3. Press the "Tab" key twice and observe the behaviour.
   Expected: Should tab through elements, all the way to the on save recs. It however skips the premium tags, which is not ideal but I scoped out of this work.

• List of other uplifts needed: None
• Risk to taking this patch: Medium
• Why is the change risky/not risky? (and alternatives if risky): It isn't a lot, mostly css, but there is a little js too.
• String changes made/needed: none

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d70d61772f1a
fixing some jank with keyboard nav on save to pocket button doorhanger r=gvn

Comment 5

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/d70d61772f1a

Comment 6

[Julien Cristau [:jcristau]]

How critical is getting this in 82? Since it's flagged S3 and medium risk, and we're going into our last week of beta, I would prefer to let it ride the trains if possible.

Comment 7

[Scott [:thecount] Downe]

For severity, I would say it's not major functionality, doesn't have a reasonable workaround, and impacts some Q4 goals.

Also when I set the risk, I had a hard time on deciding if it was low or medium. It's likely more low than medium, but I figured just to be safe put it in medium, and if there was concern, it could be discussed.

If we didn't have this patch, we would would either not launch the feature, which would be a pretty big deal to the Q$ goal, or we would launch with less than acceptable accessibility support for the feature.

If it helps, we can pref it off if there are major issues, which I think are unlikely.

I was probably being overly careful with the medium. My low might be someone else's medium, which I'm aware of, so bumped it up to medium.

Given these definitions:
Eg : Low risk because its a one line CSS change impacting only settings page
Eg : Medium, given the code complexity and integration with other areas of code that might be impacted. Expect regressions in areas like.

It's not a one line css, it's 3, and a few lines of js, but it's also not complex code, nor would it impact other areas. Plus the pref to turn it off. Maybe I was overly careful with the medium? I see an argument for it being low.

Comment 8

[Marius Coman [:mcoman], Ecosystem QA]

I have verified that this issue is no longer reproducible with the latest Firefox Nightly (83.0a1 Build ID - 20201006041051) installed, on Windows 10 x64, Ubuntu 18.04 x64, and Mac 10.15. Now I can confirm that all the elements from the "Saved to Pocket" doorhanger are visibly focused through keyboard navigation.

Comment 9

[Julien Cristau [:jcristau]]

Comment on attachment 9179095 [details]
Bug 1668247 - fixing some jank with keyboard nav on save to pocket button doorhanger

approved for 82.0b8

Comment 10

[Julien Cristau [:jcristau]]

https://hg.mozilla.org/releases/mozilla-beta/rev/1055fccff0c4

Comment 11

[Marius Coman [:mcoman], Ecosystem QA]

I have verified that this issue is no longer reproducible with the latest Firefox Beta (82.0b8 Build ID - 20201006142214) installed, on Windows 10 x64, Ubuntu 18.04 x64, and Mac 10.15. Now I can confirm that all the elements from the "Saved to Pocket" doorhanger are visibly focused through keyboard navigation.