Closed Bug 1535701 Opened 9 months ago Closed 9 months ago

Make GeckoView's focus work similar to WebView's

Categories

(GeckoView :: General, defect, P1)

Unspecified
Android
defect

Tracking

(firefox65 wontfix, firefox66 wontfix, firefox67 fixed, firefox68 fixed)

RESOLVED FIXED
mozilla68
Tracking Status
firefox65 --- wontfix
firefox66 --- wontfix
firefox67 --- fixed
firefox68 --- fixed

People

(Reporter: eeejay, Assigned: eeejay)

Details

Attachments

(3 files)

  WebView GeckoView Comment
isFocusableInTouchMode TRUE FALSE We should fix this, I think it will potentially simplify things.
touch focuses TRUE TRUE
touch unfocuses when tapping outside view FALSE FALSE
DOM focus change focuses view FALSE FALSE
TalkBack explore by touch focuses FALSE TRUE I think explore by touch should focus view
TalkBack swipe focuses FALSE FALSE I think direct user interactions with TalkBack in the GeckoView should focus view
When view is focused a11y focus event fires on DOM focus changes TRUE TRUE
When view is blured a11y focus event fires on DOM focus changes FALSE TRUE We should fix this.

About my two comments above: When TalkBack receives a focus event, it redirects the accessibility focus (the green cursor) to the focused element. This is an important driver for the screen reader experience.

Since the focus mode of the webview/geckoview is "focusable in touch", the focused state of the view is very arbitrary when using TalkBack since the user never directly touches the view. The only way for the view to regain focus is if a control or link in the content is interacted with.

I think a TalkBack user, who is directly explicitly interacting with the webview/geckoview would expect it to have focus, and to have the accessibility focus redirected in the case of script-driven focus events.

Oh! GeckoView has isFocusableInTouchMode, I don't know how I missed that.

So this turned into a purely accessibility bug. I'm fine with that.

I assume this is a P1 since have GeckoView accessibility be compatible with WebView seems important.

Eitan, feel free to change the priority as you see fit.

OS: All → Android
Priority: -- → P1

I now see this issue more about doing things differently from WebView when it comes to focus as I rationalized above. This is important to get find-in-page accessible. So I think a P1 is fitting.

This is more consistent with how it works in GeckoView. We need to give
up this behavior because it reset the top-level caret position on each
blur. This interferes with find in page which goes to the next search
result past the current caret.

When TalkBack receives a focus event, it redirects the accessibility focus (the green cursor) to the focused element. This is an important driver for the screen reader experience.

Since the focus mode of the GeckoView is "focusable in touch", the focused state of the view is very arbitrary when using TalkBack since the user never directly touches the view. The only way for the view to regain focus is if a control or link in the content is interacted with.

TalkBack user, who is explicitly interacting with the webview/geckoview would expect it to have focus, and to have the accessibility focus redirected in the page in the case of script-driven focus events.

Depends on D23746

67=affected

If we don't land in 67 Nightly before the merge deadline (2019-03-18), we might want to uplift the fix to 67 Beta. GeckoView 67 might be the version that ships in Fenix MVP.

Pushed by eisaacson@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9bf015225ed4
Don't clear focus when moving vc away from focused node. r=yzen
https://hg.mozilla.org/integration/autoland/rev/99aab1b8a0f7
Focus GeckoView when interacting with TalkBack. r=geckoview-reviewers,snorp
Pushed by eisaacson@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/93b76c2b5d62
Followup to fix checkstyle and eslint.

Eitan, do you think we should uplift this fix to GeckoView 67 Beta? The Fenix MVP release will use GeckoView 67.

Flags: needinfo?(eitan)

Yes. I want to let this cook for a bit. Maybe friday?

Flags: needinfo?(eitan)

(In reply to Eitan Isaacson [:eeejay] from comment #12)

Yes. I want to let this cook for a bit. Maybe friday?

Sure. No rush. I just wanted to get the uplift on your radar.

Comment on attachment 9051424 [details]
Bug 1535701 - Don't clear focus when moving vc away from focused node. r?yzen!

Beta/Release Uplift Approval Request

  • Feature/Bug causing the regression: None
  • User impact if declined: Find in page won't be fully accessible to TalkBack users
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: Bug 1536123
  • Risk to taking this patch: Medium
  • Why is the change risky/not risky? (and alternatives if risky): This will only affect users with accessibility services enabled.
  • String changes made/needed:
  • Do you want to request approval of these patches as well?: on, on
Attachment #9051424 - Flags: approval-mozilla-beta?
Attachment #9051425 - Flags: approval-mozilla-beta?
Attachment #9052340 - Flags: approval-mozilla-beta?

Comment on attachment 9051424 [details]
Bug 1535701 - Don't clear focus when moving vc away from focused node. r?yzen!

Fix for an accessibility issue with Talkback on Android, been on Nightly for a week and P1 for Geckoview, uplift approved for 67 beta, thanks.

Attachment #9051424 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9051425 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9052340 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.