Closed Bug 1330745 Opened 5 years ago Closed 4 years ago

When privacy.userContext.longPressBehavior is set to 1, it's impossible to open a default container tab from the + button.

Categories

(Firefox :: Tabbed Browser, defect, P3)

50 Branch
defect

Tracking

()

RESOLVED FIXED
Firefox 58
Tracking Status
firefox57 --- fix-optional
firefox58 --- fixed

People

(Reporter: baku, Assigned: jkt)

References

(Blocks 1 open bug)

Details

(Whiteboard: [userContextId][domsecurity-backlog])

Attachments

(1 file)

We need some UX here. I suggest to add an item has we did in the Context menu.
Adding STR in case someone from the community decides to verify this bug once it's fixed.

* launch the latest version of nightly
* load about:config and set the "privacy.userContext.longPressBehavior" to 1
* Click on the "+" button to open a new tab

When the container submenu appears, you'll notice that there's no way to open a regular container using this method.
Has STR: --- → yes
OS: Unspecified → All
Hardware: Unspecified → All
Whiteboard: [userContextId][domsecurity-backlog]
(In reply to Andrea Marchesini [:baku] from comment #0)
> We need some UX here. I suggest to add an item has we did in the Context
> menu.

Another possible solution is making the "+" button open a default container once it's clicked on the second time around once the container menu has appeared. Example:

* user clicks on the "+" while privacy.userContext.longPressBehavior;1
* the container menu under the "+" button instantly appears
* user clicks on the "+" the second time around and a new default tab is created

Making this a P3 as the user has to manually change the pref via about:containers.
Priority: -- → P3
Component: DOM: Security → Tabbed Browser
Product: Core → Firefox
Duplicate of this bug: 1401408
Assignee: nobody → jkt
We can change the wording to "Default Tab" or whatever in a follow up commit if we need it. However given that this isn't ever default behavior or exposed to any users I think this is fine for now.
Comment on attachment 8911787 [details]
Bug 1330745 - Add 'No Container' when longPressBehaviour is 1 for containers new tab context menu.

https://reviewboard.mozilla.org/r/183208/#review188410

::: browser/components/contextualidentity/test/browser/browser_newtabButton.js:58
(Diff revision 1)
> +  EventUtils.synthesizeMouseAtCenter(newTabButton, {type: "mousedown"});
> +  await popupShownPromise;
> +  let contextIdItems = popup.querySelectorAll("menuitem");
> +  // 4 + default + manage containers
> +  is(contextIdItems.length, 6, "Has 6 menu items");
> +  console.log(contextIdItems[0].label);

Leftover debugging code.

::: browser/components/contextualidentity/test/browser/browser_newtabButton.js:76
(Diff revision 1)
> +      // waitForNewTab doesn't work as the location doesn't change
> +      waitForTabPromise = new Promise((resolve, reject) => {
> +        gBrowser.tabContainer.addEventListener("TabOpen", function(openEvent) {
> +          resolve(openEvent.target);
> +        });
> +      });

This should use `waitForEvent`, and it's also leaking the listener right now. Why does `waitForNewTab` work in the default case, but not in the container case? The previous code only used non-default tabs and those used `waitForNewTab`...

If the `waitForEvent` code works for the default case too, just use that one?
Attachment #8911787 - Flags: review?(gijskruitbosch+bugs) → review+
waitForNewTab doesn't work in default case, the event is handled differently in the default case the waitForNewTab mentions this in their comment.
Comment on attachment 8911787 [details]
Bug 1330745 - Add 'No Container' when longPressBehaviour is 1 for containers new tab context menu.

https://reviewboard.mozilla.org/r/183208/#review189234
Attachment #8911787 - Flags: review?(amarchesini) → review+
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/193675c58378
Add 'No Container' when longPressBehaviour is 1 for containers new tab context menu. r=baku,Gijs
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/193675c58378
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
Should we uplift this to 57?
Flags: needinfo?(jkt)
No, I don't think it's worth it despite the low risk.
Flags: needinfo?(jkt)
Tanvi asked me to explain the pref values:

0 - is the default behaviour, containers menus are never created and show
1 - the menu is shown on first click, including the item "No Container" with a line separator to the other containers (exactly like how right clicking on a link does)
2 - Doesn't show the "No Container" option or separator and has a 500ms pause before showing the menu in which time the user can stop holding and instead create a "no container" tab.

There is also a line separator in the menu and "Manage Containers" as the last items in both 1,2 cases.

Also 2 will open like the back button does when you click and drag down into the menu without waiting the 500ms.
You need to log in before you can comment on or make changes to this bug.