2042925 - window.open with popup features opens as a full browser window when invoked from a Shift+click (breaks Gmail Compose popout)

Status: RESOLVED FIXED

(Core :: DOM: Window and Location, defect)

Description

[Mike Kaply [:mkaply]]

When a site calls window.open(url, name, "width=...,height=...,location=0,resizable=1,scrollbars=1") from a click handler where the user held a modifier key (Shift, Ctrl/Cmd, middle-click), Firefox ignores the popup features and opens the URL as a regular new browser window with full chrome — tab strip, bookmarks toolbar, sidebar, extensions, the lot. Chrome, Edge, and Safari respect the features and produce a minimal popup regardless of modifier keys.

The most visible casualty is Gmail's "Compose" → Shift+click "popout" affordance, which is documented Gmail behavior and works correctly in every other browser.

Root cause

nsWindowWatcher::CalculateChromeFlagsForContent (toolkit/components/windowwatcher/nsWindowWatcher.cpp:1853) checks ShouldOpenPopup(aFeatures) first — and for Gmail's features that correctly returns true. But immediately after, IsWindowOpenLocationModified(aModifiers, ...) short-circuits with CHROME_ALL whenever a modifier key was held during the originating click, discarding the site's explicit popup intent.

if (aFeatures.IsEmpty() || !ShouldOpenPopup(aFeatures)) {
  return nsIWebBrowserChrome::CHROME_ALL;
}

int32_t unused;
if (IsWindowOpenLocationModified(aModifiers, &unused)) {
  return nsIWebBrowserChrome::CHROME_ALL;   // ← clobbers site's popup intent
}

Steps to reproduce

1. In Firefox, open Gmail.
2. Shift+click the Compose button.
3. Observe: full browser window with tabs, bookmarks toolbar, sidebar.
4. Repeat in Chrome — minimal popup window.

Reduced testcase (paste into any page's content console, then Shift+click anywhere):

document.body.onclick = function(e) {
  if (e.shiftKey) window.open("https://example.com","x","width=800,height=400,location=0,resizable=1,scrollbars=1");
};

Proposed fix

Drop the modifier-key override when the site explicitly requested a popup via features. The user's modifier preference for plain links is still honored — this only changes behavior when a site's window.open features already say "open as popup."

Related

Bug 2042833 covers separate chrome-stripping cleanup (hamburger/star) inside an already-minimal popup; once this bug is fixed, the Gmail window will become a minimal popup and 2042833's fix will also apply to it.

Comment 1

[Mike Kaply [:mkaply]]

Attachment: Bug 2042925 - Don't override site-requested popup features with modifier keys. r?smaug

Comment 2

[Mike Kaply [:mkaply]]

Why this doesn't regress bug 1873330

The modifier-key check we're removing was added by bug 1873330 (Part 6, 36c7ec1d8873). That bug was about one thing: making the user's modifier keys steer where a window.open() lands (background tab / foreground tab / new window), like BrowserUtils.whereToOpenLink does for anchor clicks. The motivating testcase did not request a popup, and arai explicitly noted "the testcase doesn't request a popup" and "webpage doesn't have an ability to request a new window."

That commit wired modifiers into two functions:

1. GetWindowOpenLocation (via IsWindowOpenLocationModified) — chooses the location. This is the actual feature 1873330 wanted.
2. CalculateChromeFlagsForContent — chooses the chrome. Here it returned CHROME_ALL whenever a modifier was held, but only after ShouldOpenPopup() had already determined the site explicitly requested a popup. Nothing in 1873330 justified stripping an explicitly-requested popup down to full browser chrome; it was an incidental side effect of routing modifiers through the chrome path too.

Our fix removes the modifier check only from CalculateChromeFlagsForContent. GetWindowOpenLocation / IsWindowOpenLocationModified is untouched, so 1873330's actual behavior is fully preserved — modifiers still control where the window opens, and browser.tabs.loadInBackground / middle-click prefs are still honored. We only restore the site's requested popup chrome.

The result: Shift+click on a popup-requesting window.open now opens a minimal popup in a new window (location is still OPEN_NEWWINDOW from the untouched path) — exactly Gmail's Compose popout. This also matches Chrome/Safari, the browsers 1873330 cited as the reference for correct behavior.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

(happy to review once you have a patch. Or, ask review from #dom-core :) )

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/39c55fd70307
https://hg.mozilla.org/integration/autoland/rev/2ed9f676ca67
Don't override site-requested popup features with modifier keys. r=edgar

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/16d2da11ee28
https://hg.mozilla.org/integration/autoland/rev/a282ac415108
Revert "Bug 2042925 - Don't override site-requested popup features with modifier keys. r=edgar" for causing failures at browser_window_open_modifiers.js.

Comment 6

[Atila Butkovits]

Backed out for causing failures at browser_window_open_modifiers.js.

Backout link: https://hg.mozilla.org/integration/autoland/rev/a282ac415108

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&selectedTaskRun=BTihEzHmQ_qYN0_WVZhcRQ.0&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=2ed9f676ca67f0757fc922a4ffcabcd31d6e624d&searchStr=mochitest

Failure log: https://treeherder.mozilla.org/logviewer?job_id=570750877&repo=autoland&task=BTihEzHmQ_qYN0_WVZhcRQ.0&lineNumber=8868

Comment 7

[Mike Kaply [:mkaply]]

Tests were in a totally different directory so I didn't catch them.

Comment 8

[BMO Github Automation]

Authored by https://github.com/mkaply
https://github.com/mozilla/enterprise-firefox/commit/39c55fd703072fdc4d9a92349cd38b43bb505749
[enterprise-main] Bug 2042925 - Don't override site-requested popup features with modifier keys. r=edgar

Authored by Atila Butkovits
https://github.com/mozilla/enterprise-firefox/commit/16d2da11ee28111af0ac0147c2a2c638a693ef2f
[enterprise-main] Revert "Bug 2042925 - Don't override site-requested popup features with modifier keys. r=edgar" for causing failures at browser_window_open_modifiers.js.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/ccce0297604b
https://hg.mozilla.org/integration/autoland/rev/8bc731df2faf
Don't override site-requested popup features with modifier keys. r=edgar,arai

Comment 10

[pstanciu]

https://hg.mozilla.org/mozilla-central/rev/8bc731df2faf