Use `gBrowser.loadTabs` to open all tabs when middle-clicking a client in Synced Tabs

VERIFIED FIXED in Firefox 51

Status

()

Firefox
Sync
P1
normal
VERIFIED FIXED
7 months ago
6 months ago

People

(Reporter: kitcambridge, Assigned: kitcambridge)

Tracking

unspecified
Firefox 53
Points:
---
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox51 verified, firefox52 verified, firefox53 verified)

Details

MozReview Requests

()

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

Attachments

(1 attachment)

See bug 1288247, comment 17. We're calling `openUILinkIn` for each tab in http://searchfox.org/mozilla-central/rev/a8b5f53e7df90df655a0982e94087ee83290c22e/browser/components/syncedtabs/TabListComponent.js#127-129, while other sidebars seem to use http://searchfox.org/mozilla-central/rev/a8b5f53e7df90df655a0982e94087ee83290c22e/browser/components/places/PlacesUIUtils.jsm#967-971,993-1001.
Comment hidden (mozreview-request)
I cargo-culted this from PlacesUIUtils. Will wait to land until I can test with a wheel mouse on my desktop.

Comment 3

7 months ago
mozreview-review
Comment on attachment 8817239 [details]
Bug 1322408 - Use `gBrowser.loadTabs` to open all tabs when middle-clicking a client in Synced Tabs.

https://reviewboard.mozilla.org/r/97612/#review97966
Attachment #8817239 - Flags: review?(markh) → review+

Updated

6 months ago
Priority: -- → P1
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 6

6 months ago
Pushed by kcambridge@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/06f433fa9f12
Use `gBrowser.loadTabs` to open all tabs when middle-clicking a client in Synced Tabs. r=markh

Comment 7

6 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/06f433fa9f12
Status: NEW → RESOLVED
Last Resolved: 6 months ago
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
Comment on attachment 8817239 [details]
Bug 1322408 - Use `gBrowser.loadTabs` to open all tabs when middle-clicking a client in Synced Tabs.

This doesn't impact functionality, but it is a small UX inconsistency. I'd be comfortable letting it ride the trains, but bug 1288247 is still on Beta, so requesting uplifts just in case.

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1288247, comment 17.
[User impact if declined]: In the Synced Tabs sidebar, middle-clicking a device will open and focus each tab in turn. This is inconsistent with the bookmarks and history sidebars, which focus the first tab and load the others in the background.
[Is this code covered by automated tests?]: Yes, but automated tests use mocks; recommend manual testing to verify.
[Has the fix been verified in Nightly?]: No; should be in tomorrow's Nightly.
[Needs manual test from QE? If yes, steps to reproduce]:

1. Sign in to Sync using the same Firefox Account on two devices.
2. On the first device, open some tabs and sync.
3. On the second device, open the Synced Tabs sidebar. Verify that the first device appears, with all tabs opened in step 2.
4. On the second device, middle-click the first device name.
5. Verify that the first tab is focused, and the other tabs are loaded in the background.

[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: No.
[Why is the change risky/not risky?]: The change is isolated to a single method, and uses the same code that's already used to open multiple tabs from other sidebars (http://searchfox.org/mozilla-central/rev/f680e72cc6579f90b992b63ca14d923d2afea612/browser/components/places/PlacesUIUtils.jsm#967-971).
[String changes made/needed]: None.
Attachment #8817239 - Flags: approval-mozilla-beta?
Attachment #8817239 - Flags: approval-mozilla-aurora?

Updated

6 months ago
status-firefox51: --- → affected
status-firefox52: --- → affected
Flags: qe-verify?

Comment 9

6 months ago
Hi Florin, 
could you help verify if this issue is fixed as expected on the latest Nightly build? Thanks!
Flags: needinfo?(florin.mezei)
Kanchan, since this is a Sync related fix on Desktop, could you verify it? I can also find someone from the Cluj team if you cannot.
Flags: qe-verify?
Flags: qe-verify+
Flags: needinfo?(kkumari)
Flags: needinfo?(florin.mezei)

Comment 11

6 months ago
QA Verification: This is to confirm that this issue is fixed on latest nightly (Build ID 20161219030207).

As expected (comment#8),after middle-click the first device name, first tab gets focused and the other tabs get loaded in the background.
Thanks!
Flags: needinfo?(kkumari)
Comment on attachment 8817239 [details]
Bug 1322408 - Use `gBrowser.loadTabs` to open all tabs when middle-clicking a client in Synced Tabs.

Fix a sync issue and was verified. Beta51+ and Aurora52+. Should be in 51 beta 10.
Attachment #8817239 - Flags: approval-mozilla-beta?
Attachment #8817239 - Flags: approval-mozilla-beta+
Attachment #8817239 - Flags: approval-mozilla-aurora?
Attachment #8817239 - Flags: approval-mozilla-aurora+
Setting status flag and bug status based on comment 11. Kanchan, you can also verify this in Beta 10 on Friday (provided that the fix lands in Beta by then).
Status: RESOLVED → VERIFIED
status-firefox53: fixed → verified

Comment 14

6 months ago
This fix remove the options to use shift+middle-click to open the tabs in new window

If you are into making Synced Tabs sidebar like History and Bookmarks sidebars you can add 'Open All in Tabs'
This will enable users to use context menu on 'client' item to open all synced tabs in new tabs without having to use middle-click or ctrl-click
(In reply to tabmix.onemen from comment #14)
> If you are into making Synced Tabs sidebar like History and Bookmarks
> sidebars...

Yeah, that is a goal - could you please open a new bug for this?

Comment 16

6 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/70c74e162506
status-firefox52: affected → fixed

Comment 17

6 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/40f05c0d1f30
status-firefox51: affected → fixed

Comment 18

6 months ago
(In reply to Mark Hammond [:markh] from comment #15)
> (In reply to tabmix.onemen from comment #14)
> > If you are into making Synced Tabs sidebar like History and Bookmarks
> > sidebars...
> 
> Yeah, that is a goal - could you please open a new bug for this?

Bug 1324895

Comment 19

6 months ago
Please note that the defect fix has been tested on Treeherder builds for Aurora and Beta:

https://hg.mozilla.org/releases/mozilla-aurora/rev/70c74e162506  (Build ID  20161220074553)
https://hg.mozilla.org/releases/mozilla-beta/rev/40f05c0d1f30 (Version  51.0b10, Build ID 20161220080652)

As a final check I will do the verification again on to be released Aurora and Beta 10 (as mentioned in comment 13) and update the tracking flags of this bug consequently.

Comment 20

6 months ago
I have verified this bug fix on following released Aurora and Beta versions and found it to be working fine:

Version 	51.0b10
Build ID 	20161222080852

Version 	52.0a2
Build ID 	2016122500400
status-firefox51: fixed → verified
status-firefox52: fixed → verified
You need to log in before you can comment on or make changes to this bug.