1856109 - lost focus from sidebar after close Edit Bookmark dialog. Unable to move focus with keyboard arrow key

Status: VERIFIED FIXED

(Core :: DOM: UI Events & Focus Handling, defect)

Description

[Alice0775 White]

Steps to reproduce:

1. Open Sidebar
2. Focus a bookmark item with keyboard
3. Press Shift+F10 to open context menu
4. Choose Edit Bookmark to open Edit Bookmark dialog. And press ESC to close it
5. Try to move selection of bookmark item with keyboard arrow key

Actual results:
Unable to move focus next/previous item with keyboard arrow key

Expected Results:
Focus should go back to where it came from.
And able to move focus next/previous item with keyboard arrow key

Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=f412f38f5b6c938db981e49c1dea24f36df093f9&tochange=f41311ee4ade08929b1224c893ccf12993d32ead

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

:sefeng, since you are the author of the regressor, bug 1811129, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 2

[Alice0775 White]

[Tracking Requested - why for this release]: Lost focus. it could trap keyboard-only users.

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

The bug is marked as tracked for firefox120 (nightly). We have limited time to fix this, the soft freeze is in 9 days. However, the bug still isn't assigned.

:hsinyi, could you please find an assignee for this tracked bug? Given that it is a regression and we know the cause, we could also simply backout the regressor. If you disagree with the tracking decision, please talk with the release managers.

For more information, please visit BugBot documentation.

Comment 4

[Sean Feng [:sefeng]]

I am looking at this.

Comment 5

[Anna Yeddi [:ayeddi]]

Confirmed with today's Nightly.

:sefeng, it looks like the focus is placed on the sidebar's frame/grouping instead of the control that triggered the modal to appear. Just in case, this is the expected keyboard behavior for modal dialogs described by W3C WAI. Thank you for looking at this!

Comment 6

[Sean Feng [:sefeng]]

Attachment: Bug 1856109 - Allow <dialog> to move back to previously focused element even if they are in different BC r=emilio

Comment 7

[Donal Meehan [:dmeehan]]

:sefeng could you set a severity on this?
We can aim to include this in a dot release for Fx119

Comment 8

[Sean Feng [:sefeng]]

Yeah, let's aim that. I am doing the last try push before landing the patch

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/630864299a19
Allow <dialog> to move back to previously focused element even if they are in different BC r=emilio

Comment 10

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/42781 for changes under testing/web-platform/tests

Comment 11

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/630864299a19

Comment 12

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 13

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:sefeng, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox120 to wontfix.

For more information, please visit BugBot documentation.

Comment 14

[Natalia Csoregi [:nataliaCs]]

Backed out for causing Bug 1861451.

• backout: https://hg.mozilla.org/integration/autoland/rev/40f8ad7cb46ee58e761f16c12d5b1b1eaa55e7c1
• push: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&searchStr=linux%2C18.04%2Cx64%2Cwebrender%2Copt%2Cmochitests%2Cwith%2Cnetworking%2Con%2Csocket%2Cprocess%2Ctest-linux1804-64-qr%2Fopt-mochitest-browser-chrome-spi-nw%2Cbc5&revision=630864299a197bf416ac18d405063b7ec40c3644
• failure log: https://treeherder.mozilla.org/logviewer?job_id=433897142&repo=autoland&lineNumber=9120

[task 2023-10-26T14:37:29.073Z] 14:37:29     INFO - TEST-PASS | browser/components/places/tests/browser/browser_default_bookmark_location.js | checking if popup is closed - 
[task 2023-10-26T14:37:29.073Z] 14:37:29     INFO - Buffered messages finished
[task 2023-10-26T14:37:29.074Z] 14:37:29     INFO - TEST-UNEXPECTED-FAIL | browser/components/places/tests/browser/browser_default_bookmark_location.js | Test timed out - 
[task 2023-10-26T14:37:29.074Z] 14:37:29     INFO - Not taking screenshot here: see the one that was previously logged
[task 2023-10-26T14:37:29.075Z] 14:37:29     INFO - TEST-UNEXPECTED-FAIL | browser/components/places/tests/browser/browser_default_bookmark_location.js | Uncaught exception received from previously timed out test bound test_context_menu_link - subdialog-loaded observer not removed before the end of test
[task 2023-10-26T14:37:29.075Z] 14:37:29     INFO - Entering test bound test_change_location_panel
[task 2023-10-26T14:37:29.076Z] 14:37:29     INFO - Not taking screenshot here: see the one that was previously logged
[task 2023-10-26T14:37:29.077Z] 14:37:29     INFO - TEST-UNEXPECTED-FAIL | browser/components/places/tests/browser/browser_default_bookmark_location.js | Uncaught exception received from previously timed out test bound test_change_location_panel - at chrome://mochitests/content/browser/browser/components/places/tests/browser/head.js:481 - TypeError: can't access property "document", win is null
[task 2023-10-26T14:37:29.077Z] 14:37:29     INFO - Stack trace:
[task 2023-10-26T14:37:29.078Z] 14:37:29     INFO - clickBookmarkStar@chrome://mochitests/content/browser/browser/components/places/tests/browser/head.js:481:5
[task 2023-10-26T14:37:29.078Z] 14:37:29     INFO - test_change_location_panel@chrome://mochitests/content/browser/browser/components/places/tests/browser/browser_default_bookmark_location.js:150:9
[task 2023-10-26T14:37:29.079Z] 14:37:29     INFO - handleTask@chrome://mochikit/content/browser-test.js:1134:26
[task 2023-10-26T14:37:29.079Z] 14:37:29     INFO - _runTaskBasedTest@chrome://mochikit/content/browser-test.js:1206:18
[task 2023-10-26T14:37:29.079Z] 14:37:29     INFO - async*Tester_execTest@chrome://mochikit/content/browser-test.js:1348:14
[task 2023-10-26T14:37:29.079Z] 14:37:29     INFO - nextTest/<@chrome://mochikit/content/browser-test.js:1123:14
[task 2023-10-26T14:37:29.079Z] 14:37:29     INFO - SimpleTest.waitForFocus/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:1058:13
[task 2023-10-26T14:37:29.080Z] 14:37:29     INFO - Not taking screenshot here: see the one that was previously logged

Comment 15

[Cristian Tuns]

Backout merged to central: https://hg.mozilla.org/mozilla-central/rev/40f8ad7cb46e

Comment 16

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/42820 for changes under testing/web-platform/tests

Comment 17

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR was closed without merging

Comment 18

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 19

[Donal Meehan [:dmeehan]]

:sefeng the scheduled 119 dot release goes to build on 2023-11-06 and goes live on 2023-11-07.
This is tracked for 119 since it's a recent regression impacting accessibility.
Are you on track for fixing this and requesting beta and release uplift?
Or, is there risk and we should at least aim to get a fix out in Fx120?

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6e6cd11123b7
Allow <dialog> to move back to previously focused element even if they are in different BC r=emilio

Comment 21

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/42867 for changes under testing/web-platform/tests

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c0dc436f3e96
Fix build bustages on dom/html/HTMLDialogElement.cpp CLOSED TREE

Comment 23

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/6e6cd11123b7
https://hg.mozilla.org/mozilla-central/rev/c0dc436f3e96

Comment 24

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 25

[Donal Meehan [:dmeehan]]

Since there is a pending NI the bot won't add a comment about an uplift request.
:sefeng, adding a reminder about beta/release uplift requests depending on risk

Comment 26

[Sean Feng [:sefeng]]

Comment on attachment 9358361 [details]
Bug 1856109 - Allow <dialog> to move back to previously focused element even if they are in different BC r=emilio

Beta/Release Uplift Approval Request

• User impact if declined: User may experience unexpected <dialog> focusing behaviours. This is an regression and might be a bigger issue for accessibility users
• 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: STR is in the description of the bug
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This is not risky because the code is relatively trivial. This is about moving the focus when <dialog> is closed, so technically speaking this is not one of those important core functionalities.
• String changes made/needed:
• Is Android affected?: Yes

Comment 27

[Donal Meehan [:dmeehan]]

Setting 119 to wontfix, discussed offline and it's safer to uplift this for beta and ride the train with Fx120

Comment 28

[Dianna Smith [:diannaS]]

Comment on attachment 9358361 [details]
Bug 1856109 - Allow <dialog> to move back to previously focused element even if they are in different BC r=emilio

Approved for 120.0b7

Comment 29

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/fc75fd8191b3
https://hg.mozilla.org/releases/mozilla-beta/rev/fba3b198dced

Comment 30

[Hani Yacoub, Desktop QA]

Verified as fixed on Firefox 121.0a1 (2023-11-06) Windows 10 x64 and on Ubuntu 20.04 x64.

Comment 31

[Hani Yacoub, Desktop QA]

Verified as fixed on Firefox 120.0b7 on Windows 10 x64 and on Ubuntu 20.04 x64.