Closed Bug 1539647 Opened 5 years ago Closed 5 years ago

Unable to navigate to the '>>' button (Show more bookmarks) using the keyboard

Categories

(Firefox :: Bookmarks & History, defect)

Product:
Firefox
Component:
Bookmarks & History
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
VERIFIED FIXED
Milestone:
Firefox 68
Tracking Flags:
Tracking Status
firefox67 --- fixed
firefox68 --- verified

People

(Reporter: itiel_yn8, Assigned: Jamie)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Bug 1539647: Make the "Show more bookmarks" button navigable with the keyboard.
47 bytes, text/x-phabricator-request
pascalc
: approval-mozilla-beta+
Details | Review

STR:

  1. Enable the Bookmarks Toolbar
  2. Add enough bookmarks to the Bookmarks Toolbar so that it'd overflow and you'll see the '>>' button
  3. Ctrl+L > Tab until you get to the first item in the Bookmarks Toolbar > RightArrow key to the rightmost item

AR:
The focused item is the last visible bookmark in this toolbar, and the '>>' button is not focued and is not selectable

ER:
I'm should be able to navigate to that button and open it using the keyboard

Has STR: --- → yes
status-firefox67: --- → affected
status-firefox68: --- → affected
Component: Toolbars and Customization → Bookmarks & History
Assignee: nobody → jteh

When the Bookmarks toolbar overflows, it contains buttons with style visibility: hidden.
ToolbarKeyboardNavigator wasn't checking for this and was trying to focus these buttons, which simply failed.
We now skip over these buttons, which means that the Show more bookmarks button can now be reached.

Pushed by jteh@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/fae91b8aa3d7
Make the "Show more bookmarks" button navigable with the keyboard. r=Gijs

Backed out changeset fae91b8aa3d7 (bug 1539647) for eslint falure at /content/browser-toolbarKeyNav.js on a CLOSED TREE.

Backout link: https://hg.mozilla.org/integration/autoland/rev/b175687d4b291a8510b0d5da84bbe1edd2c5877e

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&revision=fae91b8aa3d7eb88a58918250953dcf5713fe16e&selectedJob=236778089

Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=236778089&repo=autoland&lineNumber=277

Log snippet:

[task 2019-03-28T23:41:35.844Z] New python executable in /builds/worker/checkouts/gecko/obj-x86_64-pc-linux-gnu/_virtualenvs/init/bin/python2.7
[task 2019-03-28T23:41:35.844Z] Also creating executable in /builds/worker/checkouts/gecko/obj-x86_64-pc-linux-gnu/_virtualenvs/init/bin/python
[task 2019-03-28T23:41:37.443Z] Installing setuptools, pip, wheel...done.
[task 2019-03-28T23:41:38.564Z] running build_ext
[task 2019-03-28T23:41:38.564Z] building 'psutil._psutil_linux' extension
[task 2019-03-28T23:41:38.564Z] creating build
[task 2019-03-28T23:41:38.564Z] creating build/temp.linux-x86_64-2.7
[task 2019-03-28T23:41:38.564Z] creating build/temp.linux-x86_64-2.7/psutil
[task 2019-03-28T23:41:38.564Z] x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -fPIC -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=543 -DPSUTIL_LINUX=1 -I/usr/include/python2.7 -c psutil/_psutil_common.c -o build/temp.linux-x86_64-2.7/psutil/_psutil_common.o
[task 2019-03-28T23:41:38.564Z] x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -fPIC -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=543 -DPSUTIL_LINUX=1 -I/usr/include/python2.7 -c psutil/_psutil_posix.c -o build/temp.linux-x86_64-2.7/psutil/_psutil_posix.o
[task 2019-03-28T23:41:38.564Z] x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -fPIC -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=543 -DPSUTIL_LINUX=1 -I/usr/include/python2.7 -c psutil/_psutil_linux.c -o build/temp.linux-x86_64-2.7/psutil/_psutil_linux.o
[task 2019-03-28T23:41:38.564Z] creating build/lib.linux-x86_64-2.7
[task 2019-03-28T23:41:38.564Z] creating build/lib.linux-x86_64-2.7/psutil
[task 2019-03-28T23:41:38.564Z] x86_64-linux-gnu-gcc -pthread -shared -Wl,-O1 -Wl,-Bsymbolic-functions -Wl,-Bsymbolic-functions -Wl,-z,relro -fno-strict-aliasing -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -Wl,-Bsymbolic-functions -Wl,-z,relro -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security build/temp.linux-x86_64-2.7/psutil/_psutil_common.o build/temp.linux-x86_64-2.7/psutil/_psutil_posix.o build/temp.linux-x86_64-2.7/psutil/_psutil_linux.o -o build/lib.linux-x86_64-2.7/psutil/_psutil_linux.so
[task 2019-03-28T23:41:38.564Z] building 'psutil._psutil_posix' extension
[task 2019-03-28T23:41:38.564Z] x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -fPIC -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=543 -DPSUTIL_LINUX=1 -I/usr/include/python2.7 -c psutil/_psutil_common.c -o build/temp.linux-x86_64-2.7/psutil/_psutil_common.o
[task 2019-03-28T23:41:38.564Z] x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -fPIC -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=543 -DPSUTIL_LINUX=1 -I/usr/include/python2.7 -c psutil/_psutil_posix.c -o build/temp.linux-x86_64-2.7/psutil/_psutil_posix.o
[task 2019-03-28T23:41:38.564Z] x86_64-linux-gnu-gcc -pthread -shared -Wl,-O1 -Wl,-Bsymbolic-functions -Wl,-Bsymbolic-functions -Wl,-z,relro -fno-strict-aliasing -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -Wl,-Bsymbolic-functions -Wl,-z,relro -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security build/temp.linux-x86_64-2.7/psutil/_psutil_common.o build/temp.linux-x86_64-2.7/psutil/_psutil_posix.o -o build/lib.linux-x86_64-2.7/psutil/_psutil_posix.so
[task 2019-03-28T23:41:38.564Z] copying build/lib.linux-x86_64-2.7/psutil/_psutil_linux.so -> psutil
[task 2019-03-28T23:41:38.564Z] copying build/lib.linux-x86_64-2.7/psutil/_psutil_posix.so -> psutil
[task 2019-03-28T23:41:38.564Z]
[task 2019-03-28T23:41:38.564Z] Error processing command. Ignoring because optional. (optional:packages.txt:comm/build/virtualenv_packages.txt)
[task 2019-03-28T23:47:40.148Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/base/content/browser-toolbarKeyNav.js:55:26 | ["visibility"] is better written in dot notation. (dot-notation)
[task 2019-03-28T23:47:40.148Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/browser/base/content/test/keyboard/browser_toolbarKeyNav.js:315:39 | Unexpected space before function parentheses. (space-before-function-paren)
[taskcluster 2019-03-28 23:47:40.478Z] === Task Finished ===
[taskcluster 2019-03-28 23:47:41.249Z] Unsuccessful task run with exit code: 1 completed in 628.161 seconds

Flags: needinfo?(jteh)
Flags: needinfo?(jteh)
Pushed by jteh@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5206486ce88e
Make the "Show more bookmarks" button navigable with the keyboard. r=Gijs

https://hg.mozilla.org/mozilla-central/rev/5206486ce88e

Status: NEW → RESOLVED
Closed: 5 years ago
status-firefox68: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 68

Veified fixed on latest Nightly. Thanks!

Status: RESOLVED → VERIFIED
status-firefox68: fixed → verified

James, is that a fix that would be safe and valuable to uplift to beta or can it ride the 68 train? Thanks

Flags: needinfo?(jteh)

Comment on attachment 9054107 [details]
Bug 1539647: Make the "Show more bookmarks" button navigable with the keyboard.

Beta/Release Uplift Approval Request

  • Feature/Bug causing the regression: Bug 1436086
  • User impact if declined: Users won't be able to navigate to the "Show more bookmarks" button with the keyboard.
  • 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: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Trivial, straightforward patch. Covered by automated tests. Code isolated to toolbar keyboard navigation (new in 67).
  • String changes made/needed:
Flags: needinfo?(jteh)
Attachment #9054107 - Flags: approval-mozilla-beta?

Comment on attachment 9054107 [details]
Bug 1539647: Make the "Show more bookmarks" button navigable with the keyboard.

Low risk patch with tests, approved for 67 beta 10, thanks.

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

Attachment

General

Creator:
Created:
Updated:
Size: