1364019 - need a way to know whether a new tab is just a previously closed tab being restored

Status: REOPENED

(WebExtensions :: General, enhancement, P5)

Description

[Dietrich Ayala (:dietrich)]

STR:

1. open a tab
2. check tab.id for that tab
3. close the tab
4. accel+shift+T to restore the tab
5. check tab.id for that tab

Expected: the id is the same

Actual: the id is different

This is problematic for any add-on managing tabs.

For example, I'm attempting to migrate an add-on to WebExtension that always places new tabs immediately to the right of the currently active tab.

onCreated fires for tabs that are restored. When that happens, my addon cannot tell that it's a *restored* tab and therefore shouldn't move it to the right of the selected tab.

Unless there's a way to know that the tab is being restored, not actually "new", then my add-on is not possible as a WebExtension.



NOTE: I think I didn't hit this pre-WebExtension because the behavior in the SDK for the tabs.on('open') API was that it didn't fire for restored tabs. I think that's not really correct, but did avoid this problem ;)

Comment 1

[Tomislav Jovanovic :zombie]

I don't think this is realistic, as current extensions assume tab IDs are unique and assigned on tab.onCreated event.  I think another property on Tab, or an event in browser.sessions should be a better solution for your original problem.

Comment 2

[Dietrich Ayala (:dietrich)]

There are pros and cons, sure.

Whatever the solution, I just need to be able to know that the tab is being restored, so don't treat it like a typical new tab (link, blank, external app).

A new property on Tab that made clear the source of the tab would be neat for other reasons - can do interesting things when we know whether a tab is from a link click, an external app (include source would be rad), from new tab shortcut/button, or being restored.

Either way, my add-on can't be migrated without this fix :(

Comment 3

[Dietrich Ayala (:dietrich)]

I de-solutionized the bug title.

Comment 4

[Kris Maglione [:kmag]]

I suspect that bug 1322060 should give you what you need for this.

Comment 5

[Kris Maglione [:kmag]]

Please reopen if the sessionstore API doesn't give you what you need.

Comment 6

[Dietrich Ayala (:dietrich)]

Maybe closing this prematurely.

With that new API I can now implement my add-on as a WebExtension... by writing a duplicate implementation of the browser internal management of tabs and tab ids.

Is that good?

Is that better than persisting the tab id, or adding an event property saying the new tab is actually an old tab come back to life?

Oddly, this wasn't a problem with the SDK tabs API. I don't know why.

Maybe "new tab" event wasn't sent for undo-close-tab actions? If so, that's a pretty major breaking change between SDK and WebExtension APIs. I wonder what Chrome's implementation does.

Comment 7

[Caitlin Neiman [:caitmuenster]]

Hi Dietrich, this has been added to the agenda for the June 3 WebExtensions APIs triage meeting. Would you be able to join us? 

Wiki: https://wiki.mozilla.org/Add-ons/Contribute/Triage#Archive

Agenda: https://docs.google.com/document/d/1pTNjK5i_8gHt3EeiUiu5KCUVeRcfwn7ybCPDBSx6CLM/edit#

Comment 8

[sblask]

I've got an add-on that I recently ported that sounds just like yours: https://addons.mozilla.org/en-US/firefox/addon/open-tabs-next-to-current/ The SDK did not have a problem with Ctrl-Shift-T, but it did have a problem with tabs being restored after closing and opening a tab. Webextensions have a problem with both. I have a hacky way to detect them. A proper way would be nice. In the SDK version of my add-on, I could listen to https://github.com/sblask/firefox-open-tabs-next-to-current/blob/f819174755662f5c2d9a10221fe01c7f0b44031d/index.js#L23 But there is no such thing in webextensions... Being able to identify why/where a tab was opened would be awesome for all kinds of things. I don't think I would need to listen for the session state if done properly. I also had feature requests asking for not opening a tab next tu current when clicking on `+` No problem with the SDK, but no way to do that using webextensions.

Comment 9

[Dietrich Ayala (:dietrich)]

(In reply to Caitlin Neiman (http://pronoun.is/she) from comment #7)
> Hi Dietrich, this has been added to the agenda for the June 3 WebExtensions
> APIs triage meeting. Would you be able to join us? 

Hi Caitlin! Sadly, that's 01:00 my time. But definitely we can talk about it here!

(That meeting time excludes most of the planet. You might not be getting the input you need if a meeting is required to decide these things, and is held at a time so perfectly wrong for so many people. Try alternating times, or exploring meeting-less-ness?)

My recommendation:

1. Decide if this is actually a problem in the WE world. Andy said that the scope of WE is augmenting web pages, not web browsers. Issues like this stem from augmenting the web browser.

2. Compare the onCreated event behavior to Chrome's implementation. We're basing our APIs on theirs, so should ensure that our implementation is correct.

3. Evaluate comment #3. The answer lies in some analysis of WE source on AMO. Either way, the actual decision is around balancing the number of WEs that make the transient tab.id assumption against the number and cost of lost legacy add-on migrations.

4. Model a solution that provides extensibility while keeping browser internals opaque to WE developers. Either persist tab.id, or be explicit via an event property or tab property (see comment #2). I'm usually in favor of being explicit.

Comment 10

[Dietrich Ayala (:dietrich)]

(In reply to sblask from comment #8)
> I've got an add-on that I recently ported that sounds just like yours:
> https://addons.mozilla.org/en-US/firefox/addon/open-tabs-next-to-current/

Awesome, I'll try yours :D

> I also had feature requests
> asking for not opening a tab next tu current when clicking on `+` No problem
> with the SDK, but no way to do that using webextensions.

Hm, wouldn't onCreated fire for that?

Comment 11

[sblask]

(In reply to Dietrich Ayala (:dietrich) from comment #10)

> > I also had feature requests
> > asking for not opening a tab next tu current when clicking on `+` No problem
> > with the SDK, but no way to do that using webextensions.
> 
> Hm, wouldn't onCreated fire for that?

It does, but as you said, it's not possible to know where it comes from. Undo close, session restore, click on link, click in context menu, keyboard shortcut, click on button - they all fire the same event, so I can't have different behaviour for them. But that's what people are asking for and and would be great if I could differentiate...

Comment 12

[Shane Caraveo (:mixedpuppy)]

Adding something to the tab object is reasonable.  There may be a way to do this with webnavigation in the meantime, Luca will look into that.

See "reload" in TransitionType

https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/webNavigation/TransitionType

Comment 13

[Dietrich Ayala (:dietrich)]

(In reply to Shane Caraveo (:mixedpuppy) from comment #12)
> Adding something to the tab object is reasonable.

Can you describe a bit what makes it reasonable? That would really help us bug reporters and API consumers understand the lay of the land.

Adding a new property, while it sounds fine to me, is a clear change from Chrome WE Tabs API. Clear written guidance or policy on how we evaluate API changes/additions would really make decision making in bugs like this easier.

Somewhat related, here are some places where Chrome Extensions developers are suffering the same problem. Worth reading to surface workarounds or ideas for API changes to address this.

* https://bugs.chromium.org/p/chromium/issues/detail?id=176030
* https://stackoverflow.com/questions/11005258/persistent-unique-id-for-chrome-tabs-that-lasts-between-browser-sessions

Comment 14

[Shane Caraveo (:mixedpuppy)]

(In reply to Dietrich Ayala (:dietrich) from comment #13)
> (In reply to Shane Caraveo (:mixedpuppy) from comment #12)
> > Adding something to the tab object is reasonable.
> 
> Can you describe a bit what makes it reasonable? That would really help us
> bug reporters and API consumers understand the lay of the land.

Well, something that sounds reasonable may not mean that it's something we will do, or that everyone on the team agrees it's reasonable.  In this case, this bug is still marked design-decision-needed, I think because we want to investigate if there is an existing way to achieve the goal.  If we decide this is a fine idea, it will move to design-decision-approved and probably P5, indicating we're not going to put it in our priority list at this time, but we'd accept patches.  If we decide it shouldn't be done, it'll get marked design-decision-denied.  We have meetings to discuss design-decision-needed bugs, and invite bug reporters for bugs that will be reviewed (presumably you got an invite).

IMO it's reasonable to have a simplified flag on a tab that indicates it is re-opened/reloaded/etc, essentially what (I think) webNavigation/TransitionType gives.  It would save devs some effort and may not be a big deal to implement.

> Adding a new property, while it sounds fine to me, is a clear change from
> Chrome WE Tabs API. Clear written guidance or policy on how we evaluate API
> changes/additions would really make decision making in bugs like this easier.

While it's important to provide compatibility on APIs that are shared, we should not feel obligated to freeze the APIs.  We want compatibility as much as possible for the base, but we want to move forward doing things we believe would be best for Firefox, and can be (ie. we are willing to) maintained in the long run.  Something like this, where a *new* flag may be introduced to an existing type, would require addon devs to practice some care if they want to use it but also create a cross-browser compatible addon.  Changing behavior of an existing property or api in an incompatible way would be denied (probably without discussion) unless there was simply and overwhelming case that it must change (I can only think of something like a massive security hole).

Comment 15

[Luca Greco [:rpl] [:luca] [:lgreco]]

I've checked if/how the webNavigation events can be helpful to detect if a tab is being restored and it is possible, but not particularly easy.

How about the following alternative approach? 
changing the tab details in the tabs.onCreated to mark a restored tab explicitly (e.g. an additional `restored: true` property), so that it would make it easier for an extension to detect a restored tab, but without changing the way we generate the tab ids.

---

To be a bit more specific about what I mean for "possible but not particularly easy":

An webNavigation.onCommitted event (which includes "forward_back" in the transitionQualifiers array property) is fired after a tab is restored (in my tests it has been always fired after the tabs.onCreated event).

Unfortunately, restoring a tab currently fires three onCommited events (all of them have been received after the tabs.onCreated event in my tests):

- the first one is the one that is generated when the new tab is ready to load the url
  (and the webNavigation event details contains the expected url, no transitionQualifiers and link as transitionType)

- the second one is generated because of an initial "about:blank" page loaded in the newly created content (related to "Bug 1252129 - Filter out webNavigation events related to new window initialization phase")

- the third one is generated when the restored url is finally loading in the newly created tab and we can detect that is coming from the session store (and this event is the one that contains the expected url and "forward_back" in the transitionQualifiers)

Then the webNavigation.onCommitted is also going to be fired for every navigation event that is going to happen in any tab (and when the user navigate the tab history back and forth, an event similar to the third one described above is going to be received as well).

Comment 16

[Dietrich Ayala (:dietrich)]

+1 to the explicit solution by using a new property in the event or tab data.

Luca, what do you think about future-proofing it with extensibility by naming the property something like "source" or "reason" with possible values like "restored", "reopened", "external", "new", "link". We could start with "reopened" and then consider future uses later (we have all that stuff in the places data already).

I think the future of web agents will depend heavily on being "smart" via usage context. As I mentioned at in comment #3, providing context for how/why a "tab" is created can open doors for doing interesting things.

Comment 18

[YUKI "Piro" Hiroshi]

FYI, here is a workaround on my addon Tree Style Tab, based on browser.sessions.setTabValue and browser.sessions.getTabValue:
https://github.com/piroor/treestyletab/blob/697d62cbe896ac97fbe8c8d19a41232ecec83bfb/webextensions/common/tree/base.js#L76
Because restored and duplicated tab inherits values stored by the addon, we can detect the tab is restored or duplicated. If you set any unique ID for each tab and the restored or duplicated tab has such an unique ID, you can find any other tab having the ID. When there is another existing tab with the ID, then the tab is duplicated, otherwise it is restored.