Closed Bug 1791740 Opened 2 years ago Closed 1 year ago

"Recently closed" tabs list shouldn't show hidden tabs (on Firefox View or other UI surfaces)

Categories

(Firefox :: Tabbed Browser, defect, P3)

Firefox 106
Desktop
All
defect

Tracking

()

RESOLVED WONTFIX

People

(Reporter: juraj.masiar, Unassigned)

References

(Blocks 1 open bug)

Details

(Whiteboard: [fidefe-2022-mr1-firefox-view])

Attachments

(1 obsolete file)

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:106.0) Gecko/20100101 Firefox/106.0

Steps to reproduce:

Extensions are using "browser.tabs.hide" API to perform actions without bothering user.
However, these tabs once closed are now visible in the "about:firefoxview" page.

Since hidden tabs are not meant to be visible, they shouldn't become visible after they are closed.

The Bugbug bot thinks this bug should belong to the 'Firefox::Tabbed Browser' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Component: Untriaged → Tabbed Browser

Are the same tabs visible in the other "recently closed" UI entrypoints?

Flags: needinfo?(juraj.masiar)

Yes, I can see them in the "main menu" / History / Recently closed tabs.

Now I remember, some very long time ago I've reported some of those other places as well:
bug 1493431 (this one is pretty annoying)
bug 1488168 (and this one is actually no longer reproducible)

Flags: needinfo?(juraj.masiar)

Thanks for the quick response. We'll look into this.

Blocks: firefox-view
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Unspecified → All
Hardware: Unspecified → Desktop
Summary: Firefox view (about:firefoxview) / "Recently closed" tabs list shouldn't show hidden tabs → "Recently closed" tabs list shouldn't show hidden tabs (on Firefox View or other UI surfaces)
Whiteboard: [fidefe-2022-mr1-firefox-view]
Severity: -- → S3
Priority: -- → P2
Priority: P2 → P3
Assignee: nobody → kcochrane
Status: NEW → ASSIGNED

Copying this comment from Gijs over from phabricator for further discussion:
In terms of the patch, I'm a little unsure if this is the right solution. That's in 2 different ways:

  • should we update all the surfaces using recently closed tabs (which probably really just means changing the data source so it doesn't include hidden tabs at all)? It seems odd to not show them here but continue to show them in History > Recently Closed Tabs
  • Although it's the solution called for in comment 0, if you read e.g. https://bugzilla.mozilla.org/show_bug.cgi?id=1493431#c5 from @mstriemer , I'm unsure that it's ultimately what we want to do. I'm fairly sure that the "tab group" add-ons that are currently around (like "simple tab groups", the panorama replacement add-ons, etc.) use hidden tabs to do this type of thing, and I could see that closing those tab groups, users would want to be able to recall them with recently closed tabs UI.

So effectively, I think this needs some more holistic thinking, possibly together with UX/product. We're inferring user/extension intent for this hidden tab (ie whether it would ever be of interest to the user), and that's a little tricky/dangerous.

One potential solution could be to track whether a tab has ever been non-hidden, and show it in recently closed tabs in that case. That would probably require adding some code to tabbrowser to track this, and then code to session restore to not store tabs that have never been visible, or something. It'd be worth checking in with Dão (for tabbrowser) and someone on the add-ons team (maybe zombie or rpl/luca) to check if that sounded reasonable to them - and we should check with the tab grouping add-ons and/or the addon the reporter is using whether this results in the desired outcome (the tab the reporter is talking about gets filtered out, while user-opened tabs that were put in tab groups do show up in the recently closed tabs list).

I'm very sorry for not flagging this complexity on the bug before now! 😞

Flags: needinfo?(tomica)
Flags: needinfo?(mstriemer)
Flags: needinfo?(dao+bmo)

Replying here since I want to ping folks who might not have Phab accounts.

(In reply to Kelly Cochrane [:kcochrane] from comment #6)

One potential solution could be to track whether a tab has ever been non-hidden, and show it in recently closed tabs in that case.

I wouldn't be against this, though I have two thoughts.

  1. I think the simpler "was this tab hidden when it was closed" logic might better reflect the user's intent, or at least more often. If a user closes a whole group of tabs which was currently visible, I would expect those to show up in Recently Closed, but if they right click on another currently hidden group and select close, it would be more surprising if they showed up, since those tabs don't seem as "open" to be "recently closed".

  2. As Dão mentioned on phab, our API doesn't currently support this "hidden from the start" state, we would have to implement it, and then extensions would have to update to use it when appropriate. And it seems likely that wouldn't even cover all such "appropriate" use cases, so perhaps a more direct API would fit better. For example adding a param to browser.tabs.remove for extensions to explicitly chose when the tab should or shouldn't appear in Recently Closed.

Juraj and Yuki, do you perhaps have an opinion about this and want to chime in?

Flags: needinfo?(yuki)
Flags: needinfo?(tomica)
Flags: needinfo?(juraj.masiar)

(In reply to Tomislav Jovanovic :zombie from comment #7)

Juraj and Yuki, do you perhaps have an opinion about this and want to chime in?

If Firefox closes a hidden tab with special flag and tabs.remove() supports a new extra parameter corresponding to the flag, I think it need to be notified as a part of removeInfo for listeners of tabs.onRemoved.

And addon authors will be happy if tabs.Tab provides something information corresponding to the new tab state "hidden from the start", when Firefox introduces it and does something new special behavior for those tabs.

Flags: needinfo?(yuki)
Flags: needinfo?(dao+bmo)
  • I like the idea of being able to create hidden tabs (and that those would be automatically excluded from "recent list" when closed - while still being hidden)
  • I still think closing hidden tab should be excluded as well, even in the use-case of tab-managers (since the tab was not "visibly opened")
  • the idea about flag in "tabs.remove" is pretty out of box, it sounds like overkill, but maybe it would even allow "new type of addons" that would allow users to "forget" opened tab.
Flags: needinfo?(juraj.masiar)

Gijs, do you have any followup thoughts on how to move forward with this given the above discussion?

Flags: needinfo?(mstriemer) → needinfo?(gijskruitbosch+bugs)

(In reply to Kelly Cochrane [:kcochrane] from comment #10)

Gijs, do you have any followup thoughts on how to move forward with this given the above discussion?

So I'm a bit torn. It seems like Juraj and Tomislav disagree with me about whether it makes sense to show hidden tabs in any of the recently closed tabs UI or revive them with the "undo close tab" keyboard shortcut.

I can't actually tell from Dão's comment on phabricator if he thinks my concern is valid (I mean, I originally thought he did but I guess it's not explicit and I don't want to make assumptions), and I'm not entirely sure how to read Yuki's comment either.

Yuki, does TST allow users to hide/show tabs, and close hidden tabs? Do you think that "undo close tab" should restore such tabs and/or that they should show up in Firefox View or the History > Recently closed tabs menu?

Dão, given tabs can't start out hidden right now, do you think we'd be OK to just filter out all hidden tabs from the session store closed tabs list?

I don't actually use any tab grouping/hiding extensions myself so if everyone else is of the opinion that I'm asking for needless complexity then we can totally do the simple thing first (ie make session restore just throw away closed tabs if they were hidden when they were closed) and see if that leads to any issues. Perhaps I'm just very mistaken about how users/extensions use hidden tabs these days.

Flags: needinfo?(yuki)
Flags: needinfo?(gijskruitbosch+bugs)
Flags: needinfo?(dao+bmo)

(In reply to :Gijs (he/him) from comment #11)

Yuki, does TST allow users to hide/show tabs, and close hidden tabs? Do you think that "undo close tab" should restore such tabs and/or that they should show up in Firefox View or the History > Recently closed tabs menu?
Sorry I forgot to explain the basic viewpoint of me about this bug. I had no opinion about hidden tabs should or should not listed in the list of recently closed tabs, and I just commented about what API changes look needed to follow to / simulate Firefox's new behavior (will be introduced through this bug) by addons.

TST has no unique feature to show/hide arbitrary tabs (except its automated tests to verify compatibility of behaviors around mixed hidden/visible tabs.) Its tab context menu commands (and gestures on the tab bar) basically simulates Firefox's ones including their side effects. For example, "Close Other Tabs" context menu command on TST's sidebar skips hidden tabs because Firefox also skips them. If something new tab context menu item like "Recently Closed Tabs" is added to Firefox's tab context menu, I'll try to simulate it via WebExtensions API. My previous comment just told about what APIs are needed on such scenario.

As a Firefox user, I ordinary don't use addons which control tab visibility. On my mental model, hidden tabs are same to closed (nonexistent) tabs. If there are some hidden and shown tabs and I execute a command to operate tabs, the natural behavior for me is that only visible tabs are affected. I'll be surprised if some forgotten tabs suddenly appear in the "recently closed tabs" list even if they are closed via some addon feature while tabs are hidden.

Hmm, but I agree that some people who use visible/hidden tabs to switch working tabs - some tab manager addons provide ability to switch working tab sets based on tab visibility. On such a scenario, hidden tabs may be accessible via something UI provided by the tab manager addon, and they may be closed explicitly by any user action. Such tabs looks enough to be listed in the "recently closed tabs". (This is the reason why I thought that removeInfo should have new property - tab management addons like TST may need to know a closed tab is closed implicitly or explicitly.)

I think:

  • "Recently closed tabs" menu should list tabs closed via explicit user actions. On major cases "a tab is closed with hidden state" looks enough reason to unlist the tab from "recently closed tabs".
  • But on some special cases (ex. switching working tab sets), tab's visibility and listing on "recently closed tabs" need to be controlled separately.
  • A new boolean option to indicate the tab should be listed in the "recently closed tabs" for tabs.remove() may be solution. It may be determined as "don't list" automatically if the option is not given and the tab is hidden, but explicitly given option value will always override the auto-determined result.

Of course this is just an opinion by an author of addon who don't use hidden tabs heavily.

Flags: needinfo?(yuki)

(In reply to :Gijs (he/him) from comment #11)

So I'm a bit torn. It seems like Juraj and Tomislav disagree with me about whether it makes sense to show hidden tabs in any of the recently closed tabs UI or revive them with the "undo close tab" keyboard shortcut.

I can't actually tell from Dão's comment on phabricator if he thinks my concern is valid (I mean, I originally thought he did but I guess it's not explicit and I don't want to make assumptions), and I'm not entirely sure how to read Yuki's comment either.
[...]
Dão, given tabs can't start out hidden right now, do you think we'd be OK to just filter out all hidden tabs from the session store closed tabs list?

I don't actually use any tab grouping/hiding extensions myself so if everyone else is of the opinion that I'm asking for needless complexity then we can totally do the simple thing first (ie make session restore just throw away closed tabs if they were hidden when they were closed) and see if that leads to any issues. Perhaps I'm just very mistaken about how users/extensions use hidden tabs these days.

Same here, I don't use any such extensions, but excluding all hidden tabs sounds problematic. I agree with your concern. We'd have to review a bunch of extensions to be more definitive.

Flags: needinfo?(dao+bmo)

Gijs, should we pull this out of the current sprint until we figure out a path?

Flags: needinfo?(gijskruitbosch+bugs)

(In reply to Kelly Cochrane [:kcochrane] from comment #14)

Gijs, should we pull this out of the current sprint until we figure out a path?

As per our conversation on zoom right now, I suggest we take this out of the sprint and prioritize https://bugzilla.mozilla.org/show_bug.cgi?id=1787945 instead.

To fix this bug we'd need someone to do some of the reviews on the most popular add-ons that hide tabs and then likely take on the webextensions work to provide APIs, convince webextensions to use those APIs as needed (ie switch from creating tabs and hiding them to creating hidden tabs to start with; or adding parameters to tabs.remove()), and then update the session restore code.

That's a lot of work for a relatively minor issue, for which webextensions also already have workarounds of their own (cf. bug 1493431 suggests if the tab is permanently hidden the extension could just... not close it? To release memory the extension could use discard, I believe...).

Flags: needinfo?(gijskruitbosch+bugs)
Assignee: kcochrane → nobody
Status: ASSIGNED → NEW
Attachment #9298088 - Attachment is obsolete: true

We're going to close out this bug since as Gijs pointed out, it'd be a lot of engineering work to do. We'll focus on giving users the ability to delete any they dont' want to see (bug 1787945).

Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: