Closed Bug 1442123 Opened 2 years ago Closed 2 years ago

Define "tab" payload for pushbox

Categories

(Firefox :: Sync, enhancement, P2)

enhancement

Tracking

()

RESOLVED FIXED

People

(Reporter: markh, Assigned: eoger)

References

(Blocks 1 open bug)

Details

We need to agree on what a "tab" looks like in pushbox payloads. We might choose to use the existing "tab" format used by the "tabs" collections and the existing sendtab, or may choose to try and improve it:

Pros/Cons for using the existing payload:
* pro: all clients already understand it
* con: the format is very desktop specific (it is tied to the sessionstore implementation) and only a fraction of the info is actually used (eg, the "back/forward" state is in the payload but not used.

Pros/Cons for defining a new payload:
* con: no clients understand it.
* pro: if we make a first cut at the payload be just the information that's actually used by consumers, the payload will be significantly smaller.
* con: We want to be careful to make the payload extensible for the future - eg, we may want the full back/forward stack to be included, may want to try and define the scroll position, and may want to try and include some cookies - but we want to try and avoid reving the format too many times. IOW, we should aim to define a payload that is both extensible to while still being understood by existing clients, even if they don't necessarily support the extended format.

Any thoughts on this?
Blocks: 1442133
Here is the payload of a tab record in the tabs collection:

>{
>  title: "1442123 - Define "tab" payload for pushbox"
>  urlHistory: ["https://bugzilla.mozilla.org/show_bug.cgi?id=1442123"]
>  icon: "https://bugzilla.mozilla.org/extensions/BMO/web/images/favicon.ico"
>  lastUsed: 1519929225
>}

Here's my proposal for a first version:

>{
>   title: String,
>   // Required.
>   entries: Array<String>,
>   // Required. Collection of N URLs.
>   // Corresponds to [...tab.back, ...tab.forward].
>   current: Number
>   // Optional (default = N).
>   // Index of the "current" tab URL in this.entries.
>}

The good thing about this schema is that we can put one only URL in "entries" and forget "current" altogether if we don't care about the URL history in the first implementation of this, making the payload pretty small.
We could also add "icon" since the AlertsService on Desktop allows us to specify an alert icon.

What do you think?
An alternative would be something like:

{
  entries: Array<Tab>,
  current: Number,
}

and a tab being:
{
  title: String
  url: String
}

That means that a possible future could include stuff like scroll position or cookies etc in all (or even just some, such as current) entries.
I like it, as we discussed on IRC we should probably rename the "Tab" type since we are talking about history entries inside a Tab object.
TabHistory or TabHistoryEntry would work for me!
Priority: -- → P2
Assignee: nobody → eoger
As discussed on IRC, let's add a from: <DeviceID> field here (so we can pull the name of the sender when creating the notification):

{
  from: ourDeviceId,
  entries: [{title: tab.title, url: tab.url}],
  current: 0
}
We have settled on the comment 4 schema.
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Component: Firefox Sync: Cross-client → Sync
Product: Cloud Services → Firefox
You need to log in before you can comment on or make changes to this bug.