Synced Tabs sidebar should open new tabs on middle click

VERIFIED FIXED in Firefox 51

Status

()

Firefox
Sync
P1
normal
VERIFIED FIXED
10 months ago
5 months ago

People

(Reporter: Robert Pollak, Assigned: eoger)

Tracking

47 Branch
Firefox 51
Points:
---
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox51 verified)

Details

(Whiteboard: [synced-tabs-ui])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

10 months ago
Currently, I can only left-click in the synced tabs sidebar. This loads the URL in the currently active tab. Middle-clicking a synced tab does nothing. I suggest opening the URL in a new tab on middle click.

Comment 1

10 months ago
Agreed - the other sidebars all do this. I'm actually surprised we didn't already support that.
OS: Linux → Unspecified
Hardware: x86_64 → Unspecified

Updated

10 months ago
Priority: -- → P2

Updated

9 months ago
Whiteboard: [synced-tabs-ui]

Updated

9 months ago
Priority: P2 → P1
Comment hidden (mozreview-request)
(Assignee)

Updated

9 months ago
Assignee: nobody → eoger
Status: NEW → ASSIGNED

Comment 3

9 months ago
mozreview-review
Comment on attachment 8783129 [details]
Bug 1288247 - Open synced tabs in a new tab on middle click.

https://reviewboard.mozilla.org/r/73072/#review70884

This looks fine to me, although for parity with the bookmarks sidebar, it appears we should also support a middle click on the device offering to open all the tabs for this device (with a dialog checking you really do want to open that many tabs). I'm not too bothered if that is done in this bug or another bug opened for that purpose.
Attachment #8783129 - Flags: review?(markh) → review+
Comment hidden (mozreview-request)
(Assignee)

Comment 5

9 months ago
Added shift-click/middle-button click/ctrl-click on the client item to open all the tabs from this client (didn't implement the dialog since we don't have it for bookmarks).
(Assignee)

Updated

9 months ago
Attachment #8783129 - Flags: review+ → review?(markh)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 8

9 months ago
Added a confirmation dialog when opening a lot of tabs at once.

Comment 9

9 months ago
mozreview-review
Comment on attachment 8783129 [details]
Bug 1288247 - Open synced tabs in a new tab on middle click.

https://reviewboard.mozilla.org/r/73072/#review71534

Great, thanks!

::: browser/components/syncedtabs/TabListComponent.js:121
(Diff revision 4)
>      BrowserUITelemetry.countSyncedTabEvent("open", "sidebar");
>    },
>  
> +  onOpenTabs(urls, where, params) {
> +    if(!PlacesUIUtils.confirmOpenInTabs(urls.length, this._window)) {
> +      return;

We should still count these opens in telemetry. Sadly countSyncedTabEvent doesn't take a count, so you probably need to loop.
Attachment #8783129 - Flags: review?(markh) → review+
(Assignee)

Updated

9 months ago
Keywords: checkin-needed

Comment 10

9 months ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/bbe3a00528ed
Open synced tabs in a new tab on middle click. r=markh
Keywords: checkin-needed

Comment 11

9 months ago
Pushed by archaeopteryx@coole-files.de:
https://hg.mozilla.org/integration/autoland/rev/f976df7f1eba
Open synced tabs in a new tab on middle click: Add whitespace after 'if' to make eslint happy. r=eslint-fix
Backed out for failing test_TabListComponent.js:

https://hg.mozilla.org/integration/autoland/rev/a862065d5651313075f6f20adde47446abd259b5
https://hg.mozilla.org/integration/autoland/rev/8cfff5c353c7e48dd766b30a46f55ecdd88f29ac

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=bbe3a00528ed2b0731854fafc25963f08be9218c
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=2462006&repo=autoland

16:10:36     INFO -  TEST-PASS | browser/components/syncedtabs/test/xpcshell/test_TabListComponent.js | testActions - [testActions : 130] true == true
16:10:36     INFO -  PROCESS | 8950 | *************************
16:10:36     INFO -  PROCESS | 8950 | A coding exception was thrown and uncaught in a Task.
16:10:36     INFO -  PROCESS | 8950 | Full message: TypeError: this._window.getBrowserURL is not a function
16:10:36     INFO -  PROCESS | 8950 | Full stack: onOpenTabs@resource:///modules/syncedtabs/TabListComponent.js:124:31
16:10:36     INFO -  PROCESS | 8950 | testActions@/home/worker/workspace/build/tests/xpcshell/tests/browser/components/syncedtabs/test/xpcshell/test_TabListComponent.js:131:3
16:10:36     INFO -  PROCESS | 8950 | _run_next_test@/home/worker/workspace/build/tests/xpcshell/head.js:1566:9
16:10:36     INFO -  PROCESS | 8950 | do_execute_soon/<.run@/home/worker/workspace/build/tests/xpcshell/head.js:713:9
16:10:36     INFO -  PROCESS | 8950 | _do_main@/home/worker/workspace/build/tests/xpcshell/head.js:210:5
16:10:36     INFO -  PROCESS | 8950 | _execute_test@/home/worker/workspace/build/tests/xpcshell/head.js:545:5
16:10:36     INFO -  PROCESS | 8950 | @-e:1:1
16:10:36     INFO -  PROCESS | 8950 | *************************
Flags: needinfo?(eoger)
Comment hidden (mozreview-request)
(Assignee)

Comment 14

9 months ago
Thanks!
Flags: needinfo?(eoger)
Keywords: checkin-needed

Comment 15

9 months ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/9c50e31bc8b6
Open synced tabs in a new tab on middle click. r=markh
Keywords: checkin-needed

Comment 16

9 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/9c50e31bc8b6
Status: ASSIGNED → RESOLVED
Last Resolved: 9 months ago
status-firefox51: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 51

Comment 17

6 months ago
No one notice middle click on a client select the last synced tab

TabListComponent.js > onOpenTabs call 'this._window.openUILinkIn(url, where, params);' for each url
when where is a tab we end up selecting each new tab until the last one
Depends on: 1322408
Good catch! I think bug 1322408 should cover it.
I have reproduced this bug with Nightly 50.0a1 (2016-07-20) on Windows 7 , 4 Bit 

This bug's fix is verified with latest Beta 

Build       ID    20161201172143
User     Agent    Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:51.0) Gecko/20100101 Firefox/51.0
[bugday-20161207]

Updated

5 months ago
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Verified fixed on Windows 7 x64, Ubuntu 14.04 x64 and Mac OSX 10.11 using Firefox 51 Beta 9 (buildID: 20161219140923).
status-firefox51: fixed → verified
You need to log in before you can comment on or make changes to this bug.