use one notification when sending large number of tabs

RESOLVED FIXED in Firefox 67

Status

()

defect
P2
normal
RESOLVED FIXED
3 months ago
a month ago

People

(Reporter: asa, Assigned: eoger)

Tracking

({regression})

Trunk
Firefox 67
All
Windows
Points:
---

Firefox Tracking Flags

(firefox-esr60 unaffected, firefox65 unaffected, firefox66 unaffected, firefox67 fixed)

Details

(Whiteboard: [3/6] patch waiting for review)

Attachments

(1 attachment)

Reporter

Description

3 months ago

When I send a large group of tabs from one machine to another, I get a Windows notification for every tab. We should collapse all those notifications into one for large numbers of tabs.

+Alex for PM context and prioritization.

Sending multiple tabs is currently implemented by just sending each tab individually, which explains why we get multiple notifications. I see two ways we could address this: expand the send-tab transfer format to explicitly support multiple tabs (a big effort with b/w compat concerns) or add some debouncing logic in the thing that shows the notifications (smaller, but has to be done separately for each send-tab client).

FWIW, the "old" way of sending tabs did explicitly support this - the notification is displayed at https://searchfox.org/mozilla-central/rev/ee40541496d3ad738097eebadaf4965ca1343b7a/browser/components/BrowserGlue.jsm#2862, which handles the "one tab" vs "many tabs" differently. The old system would send many but the push-based system only ever sends 1. We should probably consider this a regression.

Keywords: regression
Flags: needinfo?(adavis)

To Mark's point, this feels like a regression because we did group them previously. IMO, seems like the person who introduced the regression should resolve that.

Flags: needinfo?(adavis)

Alrighty. Seems like :eoger is most likely to have a sense of how we can resolve this - Ed, thoughts?

FWIW, this is also the sort of thing that should appear in the "this is how send-tab should work on Fenix" document we've been talking about, or we risk repeating the experience in that integration as well.

Flags: needinfo?(eoger)
Assignee

Updated

3 months ago
Flags: needinfo?(eoger)
Assignee

Comment 6

3 months ago

I've posted a patch that should help reproduce the older behaviour.
As Ryan said, incoming tabs grouping never was an explicit product requirement, so maybe we should make it for our Fenix implementation.

Loosely related but important too: In the future, we should also evolve our schema to handle multiple tabs being sent at the same time.

Assignee: nobody → eoger
Status: NEW → ASSIGNED
Priority: -- → P2

Updated

3 months ago
Whiteboard: [3/6] patch waiting for review

Edouard, do you think this will land before the final merge to beta next week? If not, should we track it for a potential beta uplift? Thanks

Flags: needinfo?(eoger)
Assignee

Comment 8

2 months ago

Yes I think we can make it before next week. I'll ping Mark on Slack today

Flags: needinfo?(eoger)

Comment 9

2 months ago
Pushed by eoger@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/fddb40ccfe5a
Debounce FxA Send Tab commands. r=markh,rfkelly

Comment 10

2 months ago
bugherder
Status: ASSIGNED → RESOLVED
Last Resolved: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 67
Type: enhancement → defect
You need to log in before you can comment on or make changes to this bug.