containers: "reopen in .." option

RESOLVED FIXED in Firefox 62

Status

()

enhancement
P3
normal
RESOLVED FIXED
2 years ago
5 months ago

People

(Reporter: dustin, Assigned: arai)

Tracking

(Depends on 2 bugs, Blocks 1 bug, {feature})

52 Branch
Firefox 62
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(relnote-firefox 62+, firefox62+ fixed)

Details

Attachments

(2 attachments, 5 obsolete attachments)

I realize there are good reasons not to move a tab between containers, but in most cases just re-opening the tab in a new container would be sufficient.  For example, if I open a github link in the default container rather than in my "dev" container, I'll notice that github does not show me logged in.

The current steps to fix this are, copy the URL, click a few times to open a new tab in the dev container, and paste the URL there.

This feature would just shortcut those steps: click -> reopen page as.. -> dev
Component: Security → Tabbed Browser
Product: Core → Firefox
Severity: normal → enhancement
Priority: -- → P3
Assignee

Comment 2

2 years ago
I really need this.
taking.
Assignee: nobody → arai.unmht
Status: NEW → ASSIGNED
Assignee

Comment 3

2 years ago
We might need to think about how this operation will be observed from WebExtensions API.
Just opening new tab with specified container and closing current tab will be fine per API, but a bit ugly.
If we change the container in place, the API might need to be updated to notify the change (I don't see cookieStoreId in tabs.update/onUpdated).
Assignee

Comment 4

2 years ago
Here's naive implementation that really reopens the tab, carrying over the following information:
  * position
  * selected
  * muted
  (is there any other thing needs to be carried over?)
and close the original tab.

How do you think about this approach?

Should we reuse the tab/browser instead of creating new one?
in that case WebExtensions API should be updated I think.
Attachment #8935022 - Flags: feedback?(jkt)
We explicitly have not done this, not because it is hard just because we are not wanting to push this path.

Tanvi should this be blocked by UX or clearing history options as we have mentioned before?

> Should we reuse the tab/browser instead of creating new one?

I'm pretty sure you can't do this currently if I understand you correctly.
Flags: needinfo?(tanvi)
Can you provide a screenshot of what the UI looks like from your patch?

See https://github.com/mozilla/multi-account-containers/issues/942 where we are trying to solve this problem in the Multi-Account Containers addon rather than the platform.

The current tab should not close automatically.  We want to provide a "reopen" option, not a "switch container type" option.  In the addon, we can achieve this by putting something in the addon created browser popup menu.  If we were to do this in the platform, I would image using the url bar and (left/right?) clicking on the name/icon of the current container.  That could have a drop down that looked like:

Reopen in...
  Personal
  Work
  Shopping
  etc.

The user could select a Container and a new tab could open with the same url but a different Container type.  (I imagine we should also support the default container or "No Container" option.  The concept of the default Container could be a little confusing.)

This needs a user experience designer.  They can review the above proposal and make modifications as they see fit.  For now, I think we should put it into Multi-Account Containers as described in the github issue and see how much it is used.  Putting it in the extension rather than platform does require a few more clicks (3 clicks instead of 2?), but its worth doing it there first before introducing it to platform.
Flags: needinfo?(tanvi)
Assignee

Comment 7

2 years ago
Posted image screenshot of the menu (obsolete) —
here's screenshot.
(clearly the 1st one is wrong that it shows "No Container" :P )

I'll fix the patch to just reopen without closing
Please make sure the tab menu item for "Reopen in Container" only appears when the about:config preference privacy.userContext.enabled is set to true.
Assignee

Comment 9

2 years ago
Now the menu just reopens the same URL in the next tab, without closing the current tab.
and also fixed the "No Container" visibility.
Attachment #8935022 - Attachment is obsolete: true
Attachment #8935022 - Flags: feedback?(jkt)
Assignee

Comment 10

Last year
looks like almost same feature can be implemented in standalone webextension, in the same tab menu (but the menu item of the current container cannot be hidden).
I'll look into the Multi-Account Containers later.
(In reply to Tooru Fujisawa [:arai] from comment #9)
> Created attachment 8939229 [details] [diff] [review]
> Add Reopen in Container tab menu.
> 
> Now the menu just reopens the same URL in the next tab, without closing the
> current tab.
> and also fixed the "No Container" visibility.

Tooru, can you provide an updated screenshot?
Comment on attachment 8939229 [details] [diff] [review]
Add Reopen in Container tab menu.

baku or jkt - can one of you take a look at this?
Attachment #8939229 - Flags: feedback?(jkt)
Attachment #8939229 - Flags: feedback?(amarchesini)
Assignee

Comment 13

Last year
(In reply to Tanvi Vyas[:tanvi] from comment #11)
> Tooru, can you provide an updated screenshot?

sure
Attachment #8936370 - Attachment is obsolete: true
f+ from me if you remove `isReopenMenu` and just use `isContextMenu`. This likely will need a rebase too.
Attachment #8939229 - Flags: feedback?(jkt) → feedback+
Assignee

Comment 15

Last year
Thank you for your feedback :)

removed isReopenMenu and rebased.

I'll add similar feature to Multi-Account Containers shortly.
Attachment #8939229 - Attachment is obsolete: true
Attachment #8939229 - Flags: feedback?(amarchesini)
Assignee

Comment 16

Last year
here's WIP patch for Multi-Account Containers:
https://github.com/arai-a/multi-account-containers/commit/d328638b10e2ebc18325b0fea1dd68b5c68452af

The difference is:
  * the menu item is shown at the botton
  * the menu item for current container is disabled, instead of hidden
  * the container icon's color is not reflected
Assignee

Comment 17

Last year
opened https://github.com/mozilla/multi-account-containers/pull/1215

(In reply to Tooru Fujisawa [:arai] from comment #16)
> here's WIP patch for Multi-Account Containers:
> https://github.com/arai-a/multi-account-containers/commit/
> d328638b10e2ebc18325b0fea1dd68b5c68452af
> 
> The difference is:
>   * the menu item for current container is disabled, instead of hidden

this one is fixed.
(now the menu items are rebuilt every time, for simplicity and as a workaround for initial run)
Comment on attachment 8974290 [details] [diff] [review]
Add Reopen in Container tab menu.

Are you able to review this one Baku? It seems fine to me, we should take it over  the MAC addon change IMO.
Attachment #8974290 - Flags: review?(amarchesini)
Comment on attachment 8974290 [details] [diff] [review]
Add Reopen in Container tab menu.

Review of attachment 8974290 [details] [diff] [review]:
-----------------------------------------------------------------

I cannot give a r+ on browser code. Gijs, can you take a look?
Attachment #8974290 - Flags: review?(gijskruitbosch+bugs)
Attachment #8974290 - Flags: review?(amarchesini)
Attachment #8974290 - Flags: review+
Comment on attachment 8974290 [details] [diff] [review]
Add Reopen in Container tab menu.

Review of attachment 8974290 [details] [diff] [review]:
-----------------------------------------------------------------

A few notes below. I don't really understand why we want to take this in Firefox rather than the add-on, but if we're convinced this needs to happen in-browser, the code looks good to me besides the comments below. :jkt, can you comment?

It would be nice to have a test though, and I'm not sure about what accesskey to use instead, so holding off on r+ to get that sorted out.

::: browser/base/content/browser.js
@@ +4510,5 @@
>  }
>  
>  /**
>   * Updates File Menu User Context UI visibility depending on
>   * privacy.userContext.enabled pref state.

Please update the comment.

@@ +4578,5 @@
> +  // Carry over some configuration.
> +  if (isSelected) {
> +    gBrowser.selectedTab = newTab;
> +  }
> +  if (currentTab.pinned) {

Pass this as a param in options instead.

@@ +4586,5 @@
> +    if (!newTab.muted) {
> +      newTab.toggleMuteAudio(currentTab.muteReason);
> +    }
> +  }
> +  gBrowser.moveTabTo(newTab, currentTab._tPos + 1);

Instead, please pass the index as part of the options you pass to `addTab`.

::: browser/locales/en-US/chrome/browser/browser.dtd
@@ +54,5 @@
>  <!ENTITY  sendToDeviceOfflineFeedback.label  "Queued (offline)">
>  <!ENTITY  moveToNewWindow.label              "Move to New Window">
>  <!ENTITY  moveToNewWindow.accesskey          "W">
> +<!ENTITY  reopenInContainer.label            "Reopen in Container">
> +<!ENTITY  reopenInContainer.accesskey        "C">

This access key conflicts with 'close tab', as far as I can tell?
Attachment #8974290 - Flags: review?(gijskruitbosch+bugs)
> I don't really understand why we want to take this in Firefox rather than the add-on, but if we're convinced this needs to happen in-browser

Mostly that we think this is a core feature for anyone using containers and not just specific to the addon itself.

:tanvi, is there anything to add here? You asked me to provide feedback on this a while ago.
Flags: needinfo?(tanvi)
The re-open in option solves a lot of usability issues when you find you are in the wrong container and hence not logged in.  You have to open another tab in the right container, copy/paste the link, and sometimes it automatically takes you to back to the original tab with "Switch to Tab".  Baking the re-open option right into the context menu shortcuts this.

From a UX perspective, it might fit nicely as a right click on the Container name in the url bar itself, but that is a bigger UI change.  This one is minor, and should only show up for users who have Containers turned on.
Assignee

Comment 23

Last year
One more reason to implement this as built-in feature:
https://github.com/mozilla/multi-account-containers/pull/1215#discussion_r190748229

WebExtension cannot open certain URLs due to security reason, so we should either open other URL for the case or hide the menu item for such tab.
If we implement this as built-in feature, such issue doesn't happen.
Assignee

Comment 24

Last year
* Updated comments
  * Fixed the addTab callsite to pass more info as parameter
  * Changed accesskey to "e", which seems not to conflict
  * Added testcase
  * Separate updateUserContextUIVisibility into updateFileMenuUserContextUIVisibility and updateTabMenuUserContextUIVisibility,
    given that the File menu item is disabled in Private Window,
    but Tab menu item should be hidden
Attachment #8974290 - Attachment is obsolete: true
I think we're good to add this to core, then. :arai, is the new patch ready for review, or does it need more work?
Flags: needinfo?(tanvi) → needinfo?(arai.unmht)
Assignee

Comment 26

Last year
Thanks!
here's updated patch (fixed testcase to pass lint)
Attachment #8982183 - Attachment is obsolete: true
Flags: needinfo?(arai.unmht)
Attachment #8983389 - Flags: review?(gijskruitbosch+bugs)
Comment on attachment 8983389 [details] [diff] [review]
Add Reopen in Container tab menu.

Review of attachment 8983389 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/base/content/browser.js
@@ +4564,5 @@
>  }
> +function updateTabMenuUserContextUIVisibility(id) {
> +  let menu = document.getElementById(id);
> +  // Visibility of Tab menu item can change frequently.
> +  menu.hidden = !Services.prefs.getBoolPref("privacy.userContext.enabled") ||

nit: please add a default to the getBoolPref call (probably false)
Attachment #8983389 - Flags: review?(gijskruitbosch+bugs) → review+

Comment 29

Last year
bugherder
https://hg.mozilla.org/mozilla-central/rev/36ced9cc9eda
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → Firefox 62
Tooru, could you do a release notes addition request please? We want this mentionned in Nightly release notes at least, thanks. https://wiki.mozilla.org/Release_Management/Release_Notes#How_to_nominate_a_bug_for_release_notes_addition.3F
Flags: needinfo?(arai.unmht)
Keywords: feature
Assignee

Comment 31

Last year
Release Note Request (optional, but appreciated)

> [Why is this notable]

as mentioned in comment #22, this fix helps the flow when one noticed that they
opened a tab in a wrong container, which often happens, when one opens the link from external application, or maybe bookmark, or in some other ways.
now they can open the page in the right container just by choosing the tab menu item, instead of copy-paste-ing the URL.

> [Affects Firefox for Android]

No

> [Suggested wording]

(not sure if this explains well, please fix the wording :)
Added "Reopen in Container" tab menu item, which allows you to open the tab in the right container when it's accidentally opened in an unexpected container.

> [Links (documentation, blog post, etc)]

None
relnote-firefox: --- → ?
Flags: needinfo?(arai.unmht)
This is in Nightly 62 release notes -- Is this general containers pref turned on for dev edition also, or just for nightly?
Flags: needinfo?(arai.unmht)
Assignee

Comment 33

Last year
this feature doesn't have specific pref, but just depends on privacy.userContext.enabled which is enabled depending on NIGHTLY_BUILD flag, so, by default this is nightly-only.
but can be enabled by pref, and also by installing Multi Account Container extension [1] which can be installed on Release as well

[1] https://addons.mozilla.org/en-US/firefox/addon/multi-account-containers/
Flags: needinfo?(arai.unmht)

Comment 34

Last year
I'm so looking forward to using this. Please forgive my ignorance, I'm running FF 60.0.2 with Containers 6.0.0. Can I enable this somehow today? Or, will I need to run a Nightly FF build?
Assignee

Comment 35

Last year
this is fixed in 62, which is currently Nightly and will be released on September

Comment 36

Last year
(In reply to Tooru Fujisawa [:arai] from comment #35)
> this is fixed in 62, which is currently Nightly and will be released on
> September

Thank you.

Updated

10 months ago
Depends on: 1485582
Added to Firefox 62 release notes.
Assignee

Updated

5 months ago
Depends on: 1511449
You need to log in before you can comment on or make changes to this bug.