1300399 - [tracking] Implement the sessions API

Status: RESOLVED FIXED

(WebExtensions :: General, defect, P3)

Description

[Manuel Reimer]

Maybe it is a bit too early to request new features that aren't covered by the Google Chrome API.

I was searching for a nice, simple, "Restore Tab" button Addon but only found unmaintained Addons which won't survive the "XPCOM to WebExtensions" switchover. But: I also didn't find the right API to do such an Addon myself...

Firefox has an function builtin to restore last closed tabs. This is easily available for XPCOM/XUL based Addons.

It would be nice to have access to this from WebExtensions. Maybe this would be even a good time to expose some more infos. Maybe a list of last closed tabs or at least a boolean to know that there are tabs to restore (to gray out the button).

Comment 1

[Andy McKay]

Its possible to the onRemoved event and keep track of them. But agreed, if the data is already there, it would be nice to expose it and save that work.

Comment 2

[Kris Maglione [:kmag]]

(In reply to Andy McKay [:andym] from comment #1)
> Its possible to the onRemoved event and keep track of them. But agreed, if
> the data is already there, it would be nice to expose it and save that work.

That's not the same thing. The undo closed tab feature restores full session store data, including session history, form data, and so forth. It also removes the restored tabs from the recently closed list.

Comment 3

[Andy McKay]

Rightio. I would recommend moving this straight to [design-decision-approved], unless you have a concern?

Comment 4

[Kris Maglione [:kmag]]

Nope, it's just a matter of designing the API. Or it might suffice to just implement https://developer.chrome.com/extensions/sessions

Comment 5

[Andy McKay]

I think a better bet is to implement the sessions API. Changing this bug to that.

Comment 6

[Bob Silverberg [:bsilverberg]]

Attachment: Bug 1300399 - Implement the sessions API

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

Comment 7

[Bob Silverberg [:bsilverberg]]

Comment on attachment 8797758 [details]
Bug 1300399 - Implement the sessions API

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

Comment 8

[Bob Silverberg [:bsilverberg]]

Comment on attachment 8797758 [details]
Bug 1300399 - Implement the sessions API

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

Comment 9

[Bob Silverberg [:bsilverberg]]

Comment on attachment 8797758 [details]
Bug 1300399 - Implement the sessions API

This isn't totally ready for review yet. There are a couple of things I need to check.

Comment 10

[Bob Silverberg [:bsilverberg]]

Comment on attachment 8797758 [details]
Bug 1300399 - Implement the sessions API

Removing the patch as it will be attached to one of the dependencies.

Comment 11

[Bob Silverberg [:bsilverberg]]

All dependencies have landed. Marking this as fixed!

Comment 12

[Will Bamberg [:wbamberg]]

I've written some docs for these: https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/sessions.

Please let me know if they're OK.

Comment 13

[Bob Silverberg [:bsilverberg]]

Thanks Will, it looks good. There were a couple of cosmetic changes that I just took care of myself. The only other thing has to do with the onChanged event.  The doc currently states "Fired when a tab or window is closed." [1], which is true, but it's not the only time that the event is fired. It's actually fired any time the list of closed windows and tabs changes, which could include a tab/window being evicted from the list because the number stored exceeds the maximum. I don't think this is something we necessarily needs to be explicit about in the documentation, but I wanted to explain it and give you an opportunity to decide whether to update the docs or not. 

[1] https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/sessions/onChanged

Comment 14

[Will Bamberg [:wbamberg]]

(In reply to Bob Silverberg [:bsilverberg] from comment #13)
> Thanks Will, it looks good. There were a couple of cosmetic changes that I
> just took care of myself.

Thanks for the review, and also for fixing my typos :).

> The only other thing has to do with the onChanged
> event.  The doc currently states "Fired when a tab or window is closed."
> [1], which is true, but it's not the only time that the event is fired. It's
> actually fired any time the list of closed windows and tabs changes, which
> could include a tab/window being evicted from the list because the number
> stored exceeds the maximum.

I think this is worth mentioning for sure, so I did. The Chrome docs just say it's fired when sessions are changed, which didn't seem very helpful, but closing tabs/windows was the only example I could think of.

Is "the maximum" the same as MAX_SESSION_RESULTS [1]?

Also, if that's the case, then... presumably this would only happen if a tab/window had been closed? Because that's the only way we would hit the maximum? If so, does onChanged fire twice in this scenario, or just once? If once, doesn't it come down to the same thing?

[1] https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/sessions/MAX_SESSION_RESULTS?

Comment 15

[Bob Silverberg [:bsilverberg]]

(In reply to Will Bamberg [:wbamberg] from comment #14)
> (In reply to Bob Silverberg [:bsilverberg] from comment #13)
> 
> > The only other thing has to do with the onChanged
> > event.  The doc currently states "Fired when a tab or window is closed."
> > [1], which is true, but it's not the only time that the event is fired. It's
> > actually fired any time the list of closed windows and tabs changes, which
> > could include a tab/window being evicted from the list because the number
> > stored exceeds the maximum.
> 
> I think this is worth mentioning for sure, so I did. The Chrome docs just
> say it's fired when sessions are changed, which didn't seem very helpful,
> but closing tabs/windows was the only example I could think of.
> 
> Is "the maximum" the same as MAX_SESSION_RESULTS [1]?
>

No, it is not. MAX_SESSION_RESULTS is just used by the API to limit the number of sessions to return from getRecentlyClosed. The maximum I'm referring to here is a Firefox pref, for example `browser.sessionstore.max_windows_undo`.

> Also, if that's the case, then... presumably this would only happen if a
> tab/window had been closed? Because that's the only way we would hit the
> maximum? If so, does onChanged fire twice in this scenario, or just once? If
> once, doesn't it come down to the same thing?
>

In that case yes, the list would only change as a result of a window being closed, and only one notification would fire, so it does amount to the same thing, which is why I felt it might not be worth documenting. There is another case, in which closed sessions may be removed because of their age, but this is an edge case that I don't think we need to document. I think simply saying it fires when the list of closed windows and/or tabs changes is enough.

Comment 16

[Will Bamberg [:wbamberg]]

> I think
> simply saying it fires when the list of closed windows and/or tabs changes
> is enough.

OK, I've done this.