1476844 - In a multi-select context, clear the selection when switching tab via keyboard shortcuts Ctrl-t/Cmd-1

Status: VERIFIED FIXED

(Firefox :: Tabbed Browser, defect, P3)

Description

[Abdoulaye O. LY]

When switching tabs with mouse click, selection is cleared. This same behavior should be applied to tab-switch with keyboard shortcuts too.

Comment 1

[Abdoulaye O. LY]

Attachment: Bug 1476844 - Clear selected tabs whenever 'TabSelect' event is fired except when switching tab from 'switchToNextMultiSelectedTab' function in /browser/base/content/tabbrowser.js.

Review commit: https://reviewboard.mozilla.org/r/258542/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/258542/

Comment 2

[Abdoulaye O. LY]

Comment on attachment 8993984 [details]
Bug 1476844 - Clear selected tabs whenever 'TabSelect' event is fired except when switching tab from 'switchToNextMultiSelectedTab' function in /browser/base/content/tabbrowser.js.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/258542/diff/1-2/

Comment 3

[Abdoulaye O. LY]

Comment on attachment 8993984 [details]
Bug 1476844 - Clear selected tabs whenever 'TabSelect' event is fired except when switching tab from 'switchToNextMultiSelectedTab' function in /browser/base/content/tabbrowser.js.

https://reviewboard.mozilla.org/r/258538/#review265570

::: browser/base/content/test/tabs/browser.ini:24
(Diff revision 2)
>  support-files =
>    test_bug1358314.html
>  [browser_isLocalAboutURI.js]
>  [browser_multiselect_tabs_active_tab_selected_by_default.js]
>  [browser_multiselect_tabs_bookmark.js]
> +[browser_multiselect_tabs_clear_selection_when_tab_switch.js]

An oversight from https://reviewboard.mozilla.org/r/255554/diff/2#index_header. None of Gijs and I saw this.

Comment 4

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8993984 [details]
Bug 1476844 - Clear selected tabs whenever 'TabSelect' event is fired except when switching tab from 'switchToNextMultiSelectedTab' function in /browser/base/content/tabbrowser.js.

https://reviewboard.mozilla.org/r/258542/#review265742

::: browser/base/content/tabbrowser.js:154
(Diff revision 2)
>  
>    _multiSelectedTabsSet: new WeakSet(),
>  
>    _lastMultiSelectedTabRef: null,
>  
> +  _clearMultiSelectionIsLocked: false,

_clearMultiSelectionLocked

::: browser/base/content/tabbrowser.js:3678
(Diff revision 2)
> -    let selectedTabs = ChromeUtils.nondeterministicGetWeakSetKeys(this._multiSelectedTabsSet)
> +      let selectedTabs = ChromeUtils.nondeterministicGetWeakSetKeys(this._multiSelectedTabsSet)
> -                                  .filter(tab => tab.isConnected && !tab.closing);
> +                                    .filter(tab => tab.isConnected && !tab.closing);
> -    let length = selectedTabs.length;
> +      let length = selectedTabs.length;
> -    gBrowser.selectedTab = selectedTabs[length - 1];
> +      gBrowser.selectedTab = selectedTabs[length - 1];
> +    }
> +    this._clearMultiSelectionIsLocked = false;

Can you wrap the above lines in a try/catch and log any exception that gets thrown? Then after the catch you can reset this to false, so we make sure the lock doesn't get stuck to true.

Comment 5

[Abdoulaye O. LY]

Comment on attachment 8993984 [details]
Bug 1476844 - Clear selected tabs whenever 'TabSelect' event is fired except when switching tab from 'switchToNextMultiSelectedTab' function in /browser/base/content/tabbrowser.js.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/258542/diff/2-3/

Comment 6

[Abdoulaye O. LY]

Comment on attachment 8993984 [details]
Bug 1476844 - Clear selected tabs whenever 'TabSelect' event is fired except when switching tab from 'switchToNextMultiSelectedTab' function in /browser/base/content/tabbrowser.js.

https://reviewboard.mozilla.org/r/258542/#review265742

Patch rebased and updated.

Comment 7

[Abdoulaye O. LY]

Hey Jared would you like to push this please!

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/697a32f394b2
Clear selected tabs whenever 'TabSelect' event is fired except when switching tab from 'switchToNextMultiSelectedTab' function in /browser/base/content/tabbrowser.js. r=jaws

Comment 9

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/697a32f394b2

Comment 10

[Mohammad Maruf Rahman]

I have reproduced this bug with Nightly 63.0a1 (2018-07-18) on Windows 10, 64 Bit!
This bug's fix is verified with latest Nightly!

Build ID 	20180815100249
User Agent 	Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:63.0) Gecko/20100101 Firefox/63.0

Comment 11

[Vlad Lucaci, Desktop QA (:vlucaci)]

Hello,

Verified fixed in latest nightly 65.0a1(2018-11-08) and latest Beta 64.0b8 on Windows 10x64, macOS 10.13 and Ubuntu 16.04x64 but only for CTRL-t combination of keys. is CTRL-1 disabled?

Comment 12

[Jared Wein [:jaws] (please needinfo? me)]

Using Ctrl-1 through Ctrl-9 should also work to clear the selection. On Mac it would be Cmd-1 through Cmd-9.

Comment 13

[Vlad Lucaci, Desktop QA (:vlucaci)]

Verified fixed with 65.0a1 (2018-11-12)and 64.0b9 on Win 10x64 and macOS 10.13.
I have noticed that the CTRL-1 through CTRL-9 does nothing on the Ubuntu platform. Should I open a separate ticket for this issue?

Comment 14

[Jared Wein [:jaws] (please needinfo? me)]

According to https://searchfox.org/mozilla-central/source/browser/base/content/browser-sets.inc#298, on Ubuntu you will need to use Alt instead of Ctrl. So Alt-1 through Alt-9 should be the equivalent Ubuntu key combinations.

Comment 15

[Vlad Lucaci, Desktop QA (:vlucaci)]

Confirming the above information from Comment 14. Closing ticket as verified fixed for Ubuntu platform as well.

thank you Jared