1869092 - Consider not allowing Left arrow key to goBack if no back button is actually on a given panelview

Status: RESOLVED WONTFIX

(Firefox :: Menus, enhancement)

Description

[Thomas Wisniewski [:twisniewski]]

The Report Broken Site feature has a confirmation panelview that it shows after reports have been sent, which has no back button.

It's confusing UX for users to be able to press the left arrow key on that panelview to get to the previous one, and it may encourage users to submit a second duplicate report when they do not have to.

Adding this code to PanelMultiView._goBack, after prevPanelView is declared seems to do the trick:

    if (!prevPanelView.node.querySelector(".subviewbutton-back")) {
      return;
    }

:gijs, does this seem like a reasonable thing to add? If so, I can submit a patch.

Comment 1

[:Gijs (he/him)]

(In reply to Thomas Wisniewski [:twisniewski] from comment #0)

The Report Broken Site feature has a confirmation panelview that it shows after reports have been sent, which has no back button.

It's confusing UX for users to be able to press the left arrow key on that panelview to get to the previous one, and it may encourage users to submit a second duplicate report when they do not have to.

Adding this code to PanelMultiView._goBack, after prevPanelView is declared seems to do the trick:

    if (!prevPanelView.node.querySelector(".subviewbutton-back")) {
      return;
    }

:gijs, does this seem like a reasonable thing to add? If so, I can submit a patch.

Something like this seems reasonable, yep!

Comment 2

[Thomas Wisniewski [:twisniewski]]

Attachment: Bug 1869092 - do not go back on left arrow key in PanelMultiViews, if the current panel has no back button; r?gijs

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d3a2a2b120e4
do not go back on left arrow key in PanelMultiViews, if the current panel has no back button; r=Gijs

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/73cc7a634df0
Fix timeouts in browser_PanelMultiView.js. r=twisniewski,test-only

Comment 5

[Cosmin Sabou [:CosminS]]

Backed out for causing osx shippable bc failures on browser_sentence_case_strings.js

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&fromchange=cdfdb226063abfec016f6ff2e1b3bdbe9e282647&tochange=89856dfbbbb09660fd0b56d3d9ba6f60c823c81f&searchStr=os%2Cx%2C11%2Cwebrender%2Cshippable%2Copt%2Cmochitests%2Cwith%2Cnetworking%2Con%2Csocket%2Cprocess%2Ctest-macosx1100-64-shippable-qr%2Fopt-mochitest-browser-chrome-spi-nw%2Cbc1&selectedTaskRun=F7w0BVJKT_2vvwbzIDZv7g.0

Failure log: https://treeherder.mozilla.org/logviewer?job_id=439869076&repo=autoland

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

[task 2023-12-13T00:10:05.202Z] 00:10:05     INFO - TEST-PASS | browser/base/content/test/static/browser_sentence_case_strings.js | Update available — restart now for appMenu-proton-update-banner should have sentence casing. - true == true - 
[task 2023-12-13T00:10:05.202Z] 00:10:05     INFO - Leaving test bound test_sentence_case_appmenu
[task 2023-12-13T00:10:05.202Z] 00:10:05     INFO - Entering test bound test_sentence_case_all_tabs_panel
[task 2023-12-13T00:10:05.203Z] 00:10:05     INFO - Global property added while loading chrome://browser/content/browser-allTabsMenu.js: TabsPanel
[task 2023-12-13T00:10:05.203Z] 00:10:05     INFO - Checking toolbarbuttons in subview with id allTabsMenu-allTabsView
[task 2023-12-13T00:10:05.203Z] 00:10:05     INFO - Checking toolbarbutton allTabsMenu-searchTabs
[task 2023-12-13T00:10:05.203Z] 00:10:05     INFO - Checking toolbarbutton allTabsMenu-containerTabsButton
[task 2023-12-13T00:10:05.203Z] 00:10:05     INFO - Checking toolbarbutton allTabsMenu-hiddenTabsButton
[task 2023-12-13T00:10:05.203Z] 00:10:05     INFO - Checking toolbarbutton 
[task 2023-12-13T00:10:05.203Z] 00:10:05     INFO - Checking toolbarbutton 
[task 2023-12-13T00:10:05.203Z] 00:10:05     INFO - Checking toolbarbutton 
[task 2023-12-13T00:10:05.204Z] 00:10:05     INFO - Checking subheaders in subview with id allTabsMenu-allTabsView
[task 2023-12-13T00:10:05.204Z] 00:10:05     INFO - Click allTabsMenu-containerTabsButton
[task 2023-12-13T00:10:05.204Z] 00:10:05     INFO - Console message: [JavaScript Warning: "browser.ui.interaction.alltabs_menu - Unknown scalar."]
[task 2023-12-13T00:10:05.204Z] 00:10:05     INFO - Checking toolbarbuttons in subview with id allTabsMenu-containerTabsView
[task 2023-12-13T00:10:05.204Z] 00:10:05     INFO - Checking toolbarbutton 
[task 2023-12-13T00:10:05.204Z] 00:10:05     INFO - Checking toolbarbutton 
[task 2023-12-13T00:10:05.204Z] 00:10:05     INFO - Checking toolbarbutton 
[task 2023-12-13T00:10:05.204Z] 00:10:05     INFO - Checking toolbarbutton 
[task 2023-12-13T00:10:05.204Z] 00:10:05     INFO - Checking toolbarbutton 
[task 2023-12-13T00:10:05.205Z] 00:10:05     INFO - Checking subheaders in subview with id allTabsMenu-containerTabsView
[task 2023-12-13T00:10:05.205Z] 00:10:05     INFO - Shown allTabsMenu-containerTabsView
[task 2023-12-13T00:10:05.205Z] 00:10:05     INFO - Buffered messages finished
[task 2023-12-13T00:10:05.205Z] 00:10:05     INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/static/browser_sentence_case_strings.js | Test timed out - 
[task 2023-12-13T00:10:05.205Z] 00:10:05     INFO - GECKO(4748) | Completed ShutdownLeaks collections in process 4748
[task 2023-12-13T00:10:05.205Z] 00:10:05     INFO - TEST-START | Shutdown

Comment 6

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

There is an r+ patch which didn't land and no activity in this bug for 2 weeks.
:twisniewski, could you have a look please?
If you still have some work to do, you can add an action "Plan Changes" in Phabricator.
For more information, please visit BugBot documentation.

Comment 7

[:Gijs (he/him)]

I'm sure Tom will get back to this next week when we're all back from break.

Comment 8

[Thomas Wisniewski [:twisniewski]]

I think the better way forward here will be fixing bug 1877814, as it make this patch unnecessary.