Closed Bug 1801957 Opened 2 years ago Closed 3 months ago

Make element picker for screenshot capture keyboard accessible

Categories

(Firefox :: Screenshots, defect, P3)

defect

Tracking

()

RESOLVED FIXED
127 Branch
Tracking Status
firefox127 --- fixed

People

(Reporter: sfoster, Assigned: niklas, NeedInfo)

References

(Blocks 2 open bugs)

Details

Attachments

(1 file)

One of the way to indicate the rectangle we want to capture on a page is to use the "element" selector. This lets the user hover over elements on the page, highlighted with an outline to provide the box coordinates to capture.

Its not clear how to make this UX keyboard accessible. Perhaps we'd need an alternate affordance like a select menu with a list of element targets (the list would be filtered to candidates with a minimum height/width etc.)

Assignee: nobody → nbaumgardner
Status: NEW → ASSIGNED
See Also: → 1874496
Blocks: 1874496
Attachment #9371114 - Attachment description: WIP: Bug 1801957 - Create region with keyboard for screenshots. r=sfoster → Bug 1801957 - Create region with keyboard for screenshots. r=sfoster

From :niklas:

The problem I'm trying to solve: There is currently no way to create a selected region in the screenshots overlay with the keyboard.
I recently added support for resizing a selected region in bug 1801954, but a user still needs to create a region with the mouse before the keyboard can be used.

I tried to look at what other browsers are doing in this scenario. Unfortunately there isn't much to go off. Most browsers, I can't even take a screenshot and the only browser that I found to have keyboard support for screenshots is MS Edge. In Edge, when you open screenshots, if your mouse isn't over the content area it will immediately move your mouse to the center of the page. If your mouse is over the content, it remains in place. Then, arrow keys will move the cursor around and you can hit space/enter to start creating a region.

I didn't like that Edge moved the mouse immediately after opening screenshots so I took a different approach.

My approach:
Screenshots will not move the mouse until an arrow key is pressed.
How it works:
If your cursor is above the content and an arrow key is pressed, the cursor will move in the direction of the arrow key that was pressed.
If your cursor is not above the content and a arrow key is pressed, the cursor will be moved to the middle of the content. Screenshots will not move the cursor until an arrow key is pressed.

When moving around the overlay with the keyboard: only hitting an arrow key will move the cursor around by 1px. If shift + arrow key, the cursor will move around by 10px.
When space is clicked while moving the cursor, it will start a region. Moving the arrow keys allows the region to sized.
When the cursor is above an element and the hover element rect is visible, hitting enter will select that region. If no hover element region exists, enter will behave the same as space.

I am also keeping the screenshots UI focused in this patch. Tab/shift + tab will keep focus to screenshots UI. Shift + F6 will escape the focus loop I've made in this patch if needed. Although, if a user has entered screenshots, it makes sense that for the current time, only screenshots UI is focusable.

:mossop, can you advise (or redirect) on the use of window.windowUtils.sendNativeMouseEvent() in the proposed patch? That gives me pause as the only other uses I see are in tests and I'm not sure if there are any particular gotchas or considerations we should be aware of.

Flags: needinfo?(dtownsend)

:morgan, do you have any comments on this approach or the usability of the result? I think it meets the stated goals in terms of making a normally-pointer driven interaction quite doable from the keyboard. I'm not aware of much prior art here, but if your team has feedback or input that would be really helpful.

Flags: needinfo?(mreschenberg)
Flags: needinfo?(dtownsend)

(In reply to Sam Foster [:sfoster] (he/him) from comment #4)

:morgan, do you have any comments on this approach or the usability of the result? I think it meets the stated goals in terms of making a normally-pointer driven interaction quite doable from the keyboard. I'm not aware of much prior art here, but if your team has feedback or input that would be really helpful.

I think the proposed UX sounds great :) it would let users who have mouse keys enabled continue to use them, since we key off of arrow key usage only. I think relying on shift as a modifier is probably well understood, we could also rely on command/control like we do for selecting tabs in ff UI -- https://support.mozilla.org/en-US/kb/keyboard-shortcuts-perform-firefox-tasks-quickly#w_selection-of-multiple-tabs

:ayeddi do you have thoughts?

Flags: needinfo?(mreschenberg) → needinfo?(ayeddi)

I like the approach selected, thus did not comment earlier.

Will the keyboard navigation pattern be added to the sumo page with the keyboard shortcuts or any other user documentation? This would be helpful for novice users and since there are very limited options where feature like this could be accessed with the keyboard, there are no set/expected keyboard interaction patterns for that too, so maybe we'd inspire others to do better by following this pattern too.

Editing to add: it seems that the comment #2 has Sam already writing the instructions off.

Flags: needinfo?(ayeddi) → needinfo?(nbaumgardner)

We have a bug on file to update the screenshots SUMO page to include the new keyboard interactions.

We can also file a bug to update the keyboard shortcuts SUMO page it you think it would be worthwhile.

Flags: needinfo?(nbaumgardner)
Pushed by nbaumgardner@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/91e7087aef18
Create region with keyboard for screenshots. r=sfoster,firefox-desktop-core-reviewers ,accessibility-frontend-reviewers,Gijs,morgan

Backed out for causing bc failures in browser_keyboard_tests.js

  • Backout link
  • Push with failures
  • Failure Log
  • Failure line: TEST-UNEXPECTED-FAIL | browser/components/screenshots/tests/browser/browser_keyboard_tests.js | Uncaught exception in test bound test_focusMovesToContentOnArrowKeydown - TypeError: screenshotsChild.overlay.cursorRegion is undefined
Flags: needinfo?(nbaumgardner)
Pushed by nbaumgardner@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b2b5c4877902
Create region with keyboard for screenshots. r=sfoster,firefox-desktop-core-reviewers ,accessibility-frontend-reviewers,Gijs,morgan
Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → 127 Branch
You need to log in before you can comment on or make changes to this bug.