Closed Bug 803675 Opened 7 years ago Closed 7 years ago

Popup always should be opened in tab on fullscreen mode

Categories

(Core :: Widget, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla21

People

(Reporter: tetsuharu, Assigned: tetsuharu)

References

Details

(Keywords: ux-userfeedback)

Attachments

(3 files, 7 obsolete files)

3.64 KB, patch
bzbarsky
: review+
Details | Diff | Splinter Review
12.96 KB, patch
bzbarsky
: review+
Details | Diff | Splinter Review
5.03 KB, patch
bzbarsky
: review+
Details | Diff | Splinter Review
I think that, On OS X Lion fullscreen mode, popup always should be opened in tab.
Because of:
* On OS X fullscreen mode, switching window is too heavy when opening popup window. On fullscreen mode, user is absorbed in app. So browser should not interrupt user.
* Popup window is related to the content which user manipulate currently. So popup should place in same screen. But currently Firefox's behavior is not. When I open a new popup window on fullscreen mode, Firefox open the new window & its window change to new full screen app's window. This is very strange.

And so, Other browsers (Chromium, Safari, Opera) open popup in tab on fullscreen mode.
Hmm, why do you think that it should be so ONLY on Mac?
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #2)
> Hmm, why do you think that it should be so ONLY on Mac?

I think that ALL platforms on desktops have same problem when using browser's fullscreen mode. This change should be reflected to all platforms.

But I made the patch for Mac because I have not test on all platform yet, and it may be disputable.
If it's able to remove #ifdef, I remove immediately it.
Attached patch wip v2 (obsolete) — Splinter Review
I remove #ifdef to apply for all desktop platform.
This work well on OSX. But I have not tested well on other platform.

And I'm working to make a test with mochitest-plain. However, I cannot detect "TabOpen" event & change browser window to fullscreen mode from test script.

Do you have any good ideas?
Attachment #673826 - Attachment is obsolete: true
Attachment #674266 - Flags: feedback?(dao)
Component: Widget: Cocoa → Widget
OS: Mac OS X → All
Summary: Lion Fullscreen: popup always should be opened in tab on OS X fullscreen mode. → Popup always should be opened in tab on fullscreen mode
> And I'm working to make a test with mochitest-plain. However, I cannot
> detect "TabOpen" event & change browser window to fullscreen mode from test
> script.
> 
> Do you have any good ideas?

I find a approach to detect them which uses mochitest-browser-chrome & window.postMessage. It may be work fine.
Attachment #674266 - Attachment is obsolete: true
Attachment #674266 - Flags: feedback?(dao)
Attachment #675939 - Flags: review?(dolske)
Attached patch part2 v1: test (obsolete) — Splinter Review
Attachment #675940 - Flags: review?(dolske)
Attachment #675939 - Flags: review?(dolske) → review?(ehsan)
Attachment #675940 - Flags: review?(dolske) → review?(ehsan)
Comment on attachment 675940 [details] [diff] [review]
part2 v1: test

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

Please move this test close to the code that it's testing.  This should not live in browser/ specifically since that way it will only run on Firefox, but the code changes are in core.

r=me with that and the comments below.

::: browser/base/content/test/browser_fullscreen-window-open.js
@@ +33,5 @@
> +  }
> +  else {
> +    // finalize
> +    // Exit browser fullscreen mode.
> +    BrowserFullScreen();

You should remove this call, and register this as a cleanup function in test() (use registerCleanupFunction) so that it always runs when the test is over, even if it times out for example and never calls finish().

@@ +130,5 @@
> +      title: "test_open_when_open_new_window_by_pref",
> +      param: "width=400,height=400",
> +    },
> +    finalizeFn: function () {
> +      Services.prefs.setIntPref(PREF_NAME, pref);

Use clearUserPref.
Attachment #675940 - Flags: review?(ehsan) → review+
Thank you for reviewing, Ehsan.
I'll clean up code with your review.

(In reply to Ehsan Akhgari [:ehsan] from comment #9)
> Please move this test close to the code that it's testing.  This should not
> live in browser/ specifically since that way it will only run on Firefox,
> but the code changes are in core.

Umm... Do you know where is the best place? I don't know about Core code well.
(In reply to comment #10)
> (In reply to Ehsan Akhgari [:ehsan] from comment #9)
> > Please move this test close to the code that it's testing.  This should not
> > live in browser/ specifically since that way it will only run on Firefox,
> > but the code changes are in core.
> 
> Umm... Do you know where is the best place? I don't know about Core code well.

I've asked Boris Zbarsky, who I think is the right reviewer.  If he's not, I'm sure we can find somebody else.  :-)
Comment on attachment 675939 [details] [diff] [review]
patch v1: window.open should open a when browser is fullscreen

Hmm, I'm pretty sure I redirected this review request yesterday, don't know what happened.  Sorry that this was left in my queue needlessly.
Attachment #675939 - Flags: review?(ehsan) → review?(bzbarsky)
Ehsan, thank you for introducing a reviewer.
Our testing story for windowwatcher and treeowners is sad...  We should probably just add some test dirs there.

As for the patch, it seems pretty weird in a multi-monitor setup, but I guess the full-screen behavior is already pretty weird there.  :(
Er, that last is only true on Mac.  What happens in multi-monitor non-Mac setups with fullscreen on one monitor?
Umm. I don't have mulch-monitor environment regardless of Mac/non-Mac. I can't confirm the behavior of fullscreen feature on multi-monitor...
That wasn't specifically a question for you, necessarily...  Does anyone here know?  I can test on Mac tomorrow, but not easily on another OS.  :(
Probably I can test it on WindowsXP on VMware. But I'm still not sure what I should do.
I tested with the patched build on WinXP in VMware.

I went to MDN and open Sign-in dialog. Then, looks like that everything behaves as he expected.

When I was into fullscreen mode in main display, the sign-in dialog is opened in the main display's window's new tab. When I was into the fullscreen mode in the other display, the sign-in dialog is opened in the second display's window's new tab.

Is this what you expected, Boris?
(In reply to Tetsuharu OHZEKI [:saneyuki_s] from comment #0)
> 
> And so, Other browsers (Chromium, Safari, Opera) open popup in tab on
> fullscreen mode.

I think that this is wrong in Windows7
At least, Google Chrome23.0.1271.97m and Opera12.11 open popup in a SEPARATE POPUP WINDOW in Windows7.

Steps to reproduce:
1. Run Google Chrome or Opera
2. Open https://developer.mozilla.org/en-US/docs/Firefox_19_for_developers
3. Enter Full Screen Mode
4. Click "Sign In" at the top-right
5. Click "Sign In" in the pulldown

Actual results:
a separate popup window appears
And IE9 also open a separate popup window.
If user clicks the opener window while the new popup window is being created or after open, the full screen opener hides the popup window. Then, user cannot switch the window with the taskbar due to in fullscreen mode. So, I believe that the new behavior is better than the current behavior.
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #22)
> If user clicks the opener window while the new popup window is being created
> or after open, the full screen opener hides the popup window. Then, user
> cannot switch the window with the taskbar due to in fullscreen mode. So, I
> believe that the new behavior is better than the current behavior.

I think just so.

At first, I only tried to fix on OSX. But I confirm that almost other desktop environment have a similar bug which is some difference from the case of OSX Lion fullscreen mode, and re-think about it.
Thus I make the current patch.
How about to add a new pref? Anyway, this bug tries to change the traditional behavior. So, even we believe the new behavior is better, but somebody may complain about the new behavior.

I guess that both files can use Preferences API. If so and it's not critical part for performance, you can use Preferences::GetBool().  If it's critical, you should use Preferences::AddBoolVarCache().
http://mxr.mozilla.org/mozilla-central/source/modules/libpref/public/Preferences.h
Comment on attachment 675939 [details] [diff] [review]
patch v1: window.open should open a when browser is fullscreen

>+  if (isFullScreen) {
>+    if (containerPref == nsIBrowserDOMWindow::OPEN_NEWWINDOW) {
>+      containerPref = nsIBrowserDOMWindow::OPEN_NEWTAB;
>+    }
>+  }

Use && here. Then, you can reduce the indent.
The "If user clicks the opener window while the new popup window is being created" problem is not specific to full-screen per se.  It sounds like the full-screen-specific issue is the taskbar one, right?  Again, how does that work in a multi-monitor setup?
(In reply to Boris Zbarsky (:bz) from comment #26)
> The "If user clicks the opener window while the new popup window is being
> created" problem is not specific to full-screen per se.  It sounds like the
> full-screen-specific issue is the taskbar one, right?

Yes. This is not only the responsibility of browser, but also the one of taskbar (or other window switcher). However, we can care its responsibility by using browser's tab-bar. I think that we should care it if we was able to catch it in our domain. Because all desktop environment cannot resolve this problem immediately.


> Again, how does that work in a multi-monitor setup?
> 

I hope that the following is the answer which you expected to me.
In multi-monitor environment, I expect to work like this:

Assumption): There are 2 monitors, (A) & (B).
-1: Make the browser window fullscreen in the monitor (A)
-2: The browser tries to open a popup window via window.open() which is called from content by user action.
-3: The browser open the new window to a new tab of the browser window which is in monitor (A).
(In reply to Boris Zbarsky (:bz) from comment #26)
> The "If user clicks the opener window while the new popup window is being
> created" problem is not specific to full-screen per se.  It sounds like the
> full-screen-specific issue is the taskbar one, right?

Yes. I meant so.

> Again, how does that work in a multi-monitor setup?

See comment 19. I'm not sure what you want me to confirm, let me know the steps.
> -3: The browser open the new window to a new tab of the browser window which is in
> monitor (A).

Yes, I know that's what your patch does.  But why is that better than opening a window on B, say?
(In reply to Boris Zbarsky (:bz) from comment #29)
> 
> Yes, I know that's what your patch does.  But why is that better than
> opening a window on B, say?

At first, for user, each of a multi-monitors is separated workspace.
And also a window of Firefox is separeted workspace. For example, per-window private browsing is a workspace which is different from normal browsing. 
These have a hierarchy like this: Monitor > (GNOME or OSX's workspace) > app window.

When the window (1) is not in full-screen mode, the window(1) will open a popup in same monitor(A). This is natural because the opener window & the opened popup are in same workspace.

When is in full-screen mode, the full-screened window(2) occupies the monitor(B) which it is in full-screen on. Thus the monitor (B) equals the workspace of its full-screened window(2). The monitor (B) has only its full-screend window(2).

Accordingly, when user is using the monitor (B), the popup opend from the window(2) should open on the monitor(B). Also, user need pays large cost when user change focusing to other monitor.   

So my patches support the above behavior.

If user opens a browser window by user action (menubar, taskbar menu), we should treat the opened browser window as other workspace from the opener window.
So window.open from chrome context is not effected by the change of patches.
I found we can reduce to check caller context in nsWindowWatcher::CalculateChromeFlags().
Attachment #692287 - Flags: review?(bzbarsky)
Change from v1:
* Add a pref to control the behavior.
* Limit to open in new tab when the caller context is chrome.
* Reflect feedback. (thank you!)
Attachment #675939 - Attachment is obsolete: true
Attachment #675939 - Flags: review?(bzbarsky)
Attachment #692289 - Flags: review?(bzbarsky)
Attached patch part-2 v2: test (obsolete) — Splinter Review
Change from v2:
* Reflect feedback (Thank you!)
Attachment #675940 - Attachment is obsolete: true
Attachment #692290 - Flags: review?(bzbarsky)
Comment on attachment 692287 [details] [diff] [review]
part-0 v1: Reduce to check the caller context

>+  if (!isCallerChrome || (isCallerChrome && !aHasChromeParent)) {

This is equivalent to:

  if (!isCallerChrome || !aHasChromeParent) {

This change is probably fine given lack of non-test support for UniversalXPConnect now...  r=me, but watch out for test regressions.
Attachment #692287 - Flags: review?(bzbarsky) → review+
Comment on attachment 692289 [details] [diff] [review]
part-1 v2: window.open from content should open a when browser is fullscreen.

Shouldn't the window watcher code here check the new preference too?

r=me with that fixed or explained.
Attachment #692289 - Flags: review?(bzbarsky) → review+
We'll need ui-review here.
Comment on attachment 692290 [details] [diff] [review]
part-2 v2: test

r=me
Attachment #692290 - Flags: review?(bzbarsky) → review+
And yes, this does need ui-review.
(In reply to Dão Gottwald [:dao] from comment #36)
> We'll need ui-review here.

Dao, Could you tell me who is an ui-reviewer ?
This is for land.

I does not request to review to Boris, I reflect only his review. (comment #34)
Attachment #692287 - Attachment is obsolete: true
Comment on attachment 692289 [details] [diff] [review]
part-1 v2: window.open from content should open a when browser is fullscreen.

I'm skeptical that this is the right thing to do. Presumably other browsers did this only for OS X and not other OSes for a reason.
Attachment #692289 - Flags: ui-review?(madhava)
(In reply to Tetsuharu OHZEKI [:saneyuki_s] from comment #41)
> try-server: https://tbpl.mozilla.org/?tree=Try&rev=013381b9faf5

I'll modify attachment 692290 [details] [diff] [review].
When we call gBrowser.selectedBrowser.contentWindow.open(), nsContentUtils::IsCallerChrome() judge it's called from chrome context.
So the testing way which I wrote is bad.
(In reply to Dão Gottwald [:dao] from comment #42)
> Comment on attachment 692289 [details] [diff] [review]
> part-1 v2: window.open from content should open a when browser is fullscreen.
> 
> I'm skeptical that this is the right thing to do. Presumably other browsers
> did this only for OS X and not other OSes for a reason.

This bug is outstanding in OSX, I agree it.

But I think that similar problems is in other desktop GUI environment. For example, on windows, if user focus to the opener window which is in full-screen, the opened popup window will hide behind the opener.

I think this is annoying for user. However, we can fix it with using browser tabbar. So I wrote the patch which target all desktop platform.

(Please see related comments, masayuki's comment #22, and my comment #30)
(In reply to Tetsuharu OHZEKI [:saneyuki_s] from comment #44)
> But I think that similar problems is in other desktop GUI environment. For
> example, on windows, if user focus to the opener window which is in
> full-screen, the opened popup window will hide behind the opener.

... just like the full-screen window hides everything else on the desktop. The user can deal with that by leaving full-screen mode or using Alt-Tab.
By the way, does this patch have any implications for DOM fullscreen mode, where we don't have a tab bar?
(In reply to Dão Gottwald [:dao] from comment #46)
> By the way, does this patch have any implications for DOM fullscreen mode,
> where we don't have a tab bar?

I confirmed behaviors in below list. I seem this change is not destructive.

Firefox's current behavior of window.open called from DOM fullscreen mode:
* Without user operating, it will be blocked by popup blocking.
* With user operating, it will works normally. DOM fullscreen mode will end.
If it's called with specifying window size, it will open in new window, and the opener content will be in DOM fullscreen mode as it is on OSX. On windows, the popup is opened behind the opener browser window (This might be bug), and DOM fullscreen mode will end.

Firefox's new behavior changed by these patches:
* Without user operating, it will be blocked by popup blocking. (This is not changed)
* With user operating, even if we specfy window size, the popup will be opened in a new tab. And DOM fullscreen mode will end.

------
It is the fact that we need to discuss about the browser UI which is related to DOM fullscreen mode. But I feel it isn't in this time. And also, I'm interested in that "how often window.open will be called when is in DOM fullscreen mode ?".
(In reply to Dão Gottwald [:dao] from comment #45)
>
> ... just like the full-screen window hides everything else on the desktop.
> The user can deal with that by leaving full-screen mode or using Alt-Tab.

Well, Alt-Tab is also useful keyboard shortcut. But keyboard shortcut is it. Its operation does not complete in pointing device. (This may be a liking,) I think it's not good. If we are able to provide easily, we should provide a way which complete in one device.
(In reply to Boris Zbarsky (:bz) from comment #35)
> Comment on attachment 692289 [details] [diff] [review]
> part-1 v2: window.open from content should open a when browser is fullscreen.
> 
> Shouldn't the window watcher code here check the new preference too?


Try-server: https://tbpl.mozilla.org/?tree=Try&rev=ee12ef1069fc
From talos results, I seem there is no performance regression.
(In reply to Tetsuharu OHZEKI [:saneyuki_s] from comment #47)
> (In reply to Dão Gottwald [:dao] from comment #46)
> > By the way, does this patch have any implications for DOM fullscreen mode,
> > where we don't have a tab bar?
> 
> I confirmed behaviors in below list. I seem this change is not destructive.
> 
> Firefox's current behavior of window.open called from DOM fullscreen mode:
> * Without user operating, it will be blocked by popup blocking.
> * With user operating, it will works normally. DOM fullscreen mode will end.
> If it's called with specifying window size, it will open in new window, and
> the opener content will be in DOM fullscreen mode as it is on OSX. On
> windows, the popup is opened behind the opener browser window (This might be
> bug), and DOM fullscreen mode will end.

Bug 724554 makes it so that this doesn't exit DOM fullscreen mode.

> Firefox's new behavior changed by these patches:
> * Without user operating, it will be blocked by popup blocking. (This is not
> changed)
> * With user operating, even if we specfy window size, the popup will be
> opened in a new tab. And DOM fullscreen mode will end.

Redirecting the popup to a tab and exiting DOM fullscreen mode (which is inevitable because we don't allow opening and switching tabs in DOM fullscreen mode) doesn't make sense, since exiting fullscreen mode renders your motivation for redirecting the popup moot.
(In reply to Dão Gottwald [:dao] from comment #51) 
> Bug 724554 makes it so that this doesn't exit DOM fullscreen mode.

Thank you for showing here.

> Redirecting the popup to a tab and exiting DOM fullscreen mode (which is
> inevitable because we don't allow opening and switching tabs in DOM
> fullscreen mode) doesn't make sense, since exiting fullscreen mode renders
> your motivation for redirecting the popup moot.

Switching tabs in DOM fullscreen mode is difficult problem…  For almost other browsers, they are not completed to implement the UI behavior DOM fullscreen mode. We need to find the good behavior by yourself.

Now, I quote the behavior of Chromium which run on OSX. This is very interesting behavior when we consider the planing to implement the UI behavior of DOM fullscreen mode. Chromium on OSX shows its UI toolbar up if user move the mouse cursor to the top end of window. (I couldn't confirm this behavior on windows.)  I think that this behavior is more natural for tab-browser than no switching tabs in DOM fullscreen mode.

And I don't understand the description your said. I write about all situations.
* If we'll don't allow opening tabs, and
** if we'll don't allow all calling window.open() in DOM fullscreen mode, then Firefox should not open any tabs in it. In this case, We need not mind about the popup redirection because there is no probability opening new tabs from DOM fullscreen mode.
** Or if we'll don't allow opening any *tabs* but we allow opening new window, we should assume that all popups from DOM fullscreen are opened in new window. This case is a reproduction of problem which we discussed on this bug.
** Or if we'll don't allow opening any *tabs*, but allow opening new window and open all popups from DOM fullscreen as modal-dialog window, it just doesn't make sense, since user will be not able to control the opener content which is in DOM  fullscreen.
Attachment #692695 - Flags: review?(bzbarsky) → review+
Madhava, could you do ui-review about this?
Yeeesh, there are a lot of combinations here. OS X vs Windows, browser fullscreen (F11) vs DOM fullscreen, multimonitor or not.

I think multimonitor is a red herring. Opened windows should always be on the same monitor. (Consider people splitting tasks by monitor, or giving a presentation on a projection display). I know we've had bugs on this in the past, and wouldn't be surprised if there are is still some weirdness, but I don't see any mentioned here.

On Windows, current Nightly works as I'd expect. No matter what mode the parent window is in (regular, maximized, F11-fullscreen) the child window opens as a regular window on top of it. What's the rationale for changing this?

On OS X, fullscreen is different (at least for 10.7+, so yet another combination?). Current Nightly is clearly wrong here -- the child briefly opens on top for a moment, and then it animates into full-screen mode. AIUI the OS X fullscreen mode (as provided by the OS) is intended to provide a more tablet/app-like experience... So, yeah, seems like child content windows should open in a new tab.  Chrome windows don't really fit this model at all, long-term Firefox and addons will have to adapt. In the interm, opening a new chrome window should problem simply exit FS mode.

In DOM fullscreen mode, seems like content should be blocked from opening a window entirely (not open it in a tab). I'd be surprised if anything was even trying to do this today -- other than popup ads -- as the interaction model seems bizarre. Chrome windows should probably do the same, although I can see an argument for allowing some modal dialogs (eg open/save file, print) to open over the full-screen content.


I'd much prefer to ensure that the default behavior is correct and avoid adding any prefs. However -- we have talked about having a "single window browsing mode"... Primarily as a consequence of integrating with tablet UI, and possibly as a feature for people who simply prefer to avoid having extra windows being created. If we're going to have optional behavior, implementing/extending the "single window mode" would be preferable.


I'm not quite sure how the current patch matches the above -- sounds like it differs by still opening a tab even when in DOM fullscreen mode, and also has an effect on Windows?
(In reply to Justin Dolske [:Dolske] from comment #54)
> On Windows, current Nightly works as I'd expect. No matter what mode the
> parent window is in (regular, maximized, F11-fullscreen) the child window
> opens as a regular window on top of it. What's the rationale for changing
> this?
> 

I think this is the problem of gui desktop environment. Basically, the fullscreen mode is presented as single-window experience. But almost desktop enviroments does not handle as it. Please see comment #44. I wrote similar accounts.

> In DOM fullscreen mode, seems like content should be blocked from opening a
> window entirely (not open it in a tab). I'd be surprised if anything was
> even trying to do this today -- other than popup ads -- as the interaction
> model seems bizarre. Chrome windows should probably do the same, although I
> can see an argument for allowing some modal dialogs (eg open/save file,
> print) to open over the full-screen content.

In the first place, I doubt that the case the new window will be opened as popup by window.open() in DOM fullscreen mode…  Does the developer expect expect such cases? (It might be some exception. Of course we should care them. But I don't think that we should not care on this bug). 
So we need to consider the browser UI for DOM fullsreen mode. I agree it.
But I don't think that it should be on the other bug/ML.
And I think these patch of this bug is not destructive for DOM fullscreen mode.

> I'd much prefer to ensure that the default behavior is correct and avoid
> adding any prefs. However -- we have talked about having a "single window
> browsing mode"... Primarily as a consequence of integrating with tablet UI,
> and possibly as a feature for people who simply prefer to avoid having extra
> windows being created. If we're going to have optional behavior,
> implementing/extending the "single window mode" would be preferable.

Yes. Extending the implementation of "single window browsing mode" is some truth.
However, many users do not use its mode. This bug is aimed to take care of users who not using  "single window browsing mode". Thus these patches which add the new pref is convenience by design.

> I'm not quite sure how the current patch matches the above -- sounds like it
> differs by still opening a tab even when in DOM fullscreen mode, and also
> has an effect on Windows?

Yes. The behavior which opening a tab even when in DOM fullscreen mode you indicate has the effect on all desktop platforms. This has some "by design". But I don't think its is not very destructive, and it's clear for user to indicate the new "popup" is opened. Tab bar can indicate about it certainly.
(In reply to Tetsuharu OHZEKI [:saneyuki_s] from comment #55)
> So we need to consider the browser UI for DOM fullsreen mode. I agree it.
> But I don't think that it should be on the other bug/ML.
> And I think these patch of this bug is not destructive for DOM fullscreen
> mode.

There are a couple of reasonable options, such as opening popups in front of the full-screen window (this is post-bug 724554 behavior, I believe) or opening them in front and leaving DOM full-screen mode (pre-bug 724554 behavior). Leaving DOM FS mode and redirecting the popup to a tab would be nonsensical and a regression.
Comment on attachment 692289 [details] [diff] [review]
part-1 v2: window.open from content should open a when browser is fullscreen.

Sticking an r- in here for the DOM FS regression.

I also agree with dolske that the manual FS mode behavior change should be limited to OS X Lion, but I suppose ultimately that's a question for ui-review.
Attachment #692289 - Flags: review-
(In reply to Justin Dolske [:Dolske] from comment #54)
> In DOM fullscreen mode, seems like content should be blocked from opening a
> window entirely (not open it in a tab).

If we want to take this route, we should get this added to the spec, such that we can get cross-browser consistency and people won't blame Firefox for misbehaving.
(In reply to Dão Gottwald [:dao] from comment #56)
> There are a couple of reasonable options, such as opening popups in front of
> the full-screen window (this is post-bug 724554 behavior, I believe) or
> opening them in front and leaving DOM full-screen mode (pre-bug 724554
> behavior). Leaving DOM FS mode and redirecting the popup to a tab would be
> nonsensical and a regression.

I think that the pre/post behavior your said cannot resolve the problem of popup window's overlap in fullscreen mode (masayuki wrote in comment #22) not only normal one but also DOM one.

Even if we accept the pre/post behavior your said and we try to resolve the problem of popup window's overlap in fullscreen mode, we'll need to open the popup as modal window. But this'll break up the behavior of window.open().

How about do you think?
(In reply to Tetsuharu OHZEKI [:saneyuki_s] from comment #55)

> I think this is the problem of gui desktop environment. Basically, the
> fullscreen mode is presented as single-window experience. But almost desktop
> enviroments does not handle as it. Please see comment #44. I wrote similar
> accounts.

I agree that this is a basic flaw any GUI environment that supports overlapping windows. Doesn't matter what size the window is (normal, maximized, or FS), you can always "lose" another window behind it. I don't think it's an effective us of time to try to fix this on Windows. As long as a newly-opened window is on top, we're good.

So let's make this patch OS X only.

> However, many users do not use its mode. This bug is aimed to take care of
> users who not using  "single window browsing mode". Thus these patches which
> add the new pref is convenience by design.

Having too many prefs decreases usability and discoverability.

> Yes. The behavior which opening a tab even when in DOM fullscreen mode you
> indicate has the effect on all desktop platforms. This has some "by design".
> But I don't think its is not very destructive, and it's clear for user to
> indicate the new "popup" is opened. Tab bar can indicate about it certainly.

I think this just results in windows unexpectedly opening in tabs. I'd much prefer to limit that behavior to either an explicit single-window-browsing mode, or in cases (like OS X) where full-screen simply doesn't work with multiple windows. DOM FS is a special case, where we should either block (preferred) or exit DOM FS mode to show the window.
(In reply to Justin Dolske [:Dolske] from comment #60)
> I agree that this is a basic flaw any GUI environment that supports
> overlapping windows. Doesn't matter what size the window is (normal,
> maximized, or FS), you can always "lose" another window behind it. I don't
> think it's an effective us of time to try to fix this on Windows. As long as
> a newly-opened window is on top, we're good.

Yes. We'll always "lose" another window behind it in all window state (normal, maximized, or FS). But in FS is especially because user cannot get window switcher (e.g. Windows' taskbar) immediately in its mode of almost desktop environments.

Sure. This change may be result unexpectedly for user and may cause user's confuse. However, I think the current behavior is not designed fully good. As the result of this designed, we are paying needless bill for the current behavior. I think this patch can resolve it.

Well then, I propose that, we'll introduce "browser.link.open_newwindow.disabled_in_fullscreen" for managing whether opening a new window via window.open if browser is in fullscreen mode. And we'll set "true" it only on OS X, and set "false" on other desktop platforms.
If there are too many prefs, we should clean up other prefs which are meaningless in this time, I think. (e.g. Does window.open() "menubar" flags need in this time?)

If we accept this proposal, I can attach the new patch immediately.
(In reply to Tetsuharu OHZEKI [:saneyuki_s] from comment #61)
>(e.g. Does window.open() "menubar" flags need in this time?)

Supplement: This flag is managed by "dom.disable_window_open_feature.menubar". (See: https://developer.mozilla.org/en-US/docs/DOM/window.open)
(In reply to Tetsuharu OHZEKI [:saneyuki_s] from comment #61)
> Well then, I propose that, we'll introduce
> "browser.link.open_newwindow.disabled_in_fullscreen" for managing whether
> opening a new window via window.open if browser is in fullscreen mode. And
> we'll set "true" it only on OS X, and set "false" on other desktop platforms.
> If there are too many prefs, we should clean up other prefs which are
> meaningless in this time, I think. (e.g. Does window.open() "menubar" flags
> need in this time?)

I'm not sure what you're proposing here. Are you referring to manual or DOM fullscreen mode? Dolske was referring to the latter. Either way, window.open being disabled on OS X while working on other platforms would be a pitfall that I think we should avoid. To get platform parity, I'd prefer exiting DOM fullscreen mode and opening the popup.
Dao:

There are three fullscreen modes. One is the DOM fullscreen mode which is caused by website script. Next is user specified fullscreen mode with F11 key. The last is OS X native fullscreen mode.

I think that the original suggestion of this bug is for the last one. But I think that in the second case should behave same as the last case since both cases cannot change windows by pointing devices.
(In reply to Dão Gottwald [:dao] from comment #63)
> I'm not sure what you're proposing here. Are you referring to manual or DOM
> fullscreen mode? Dolske was referring to the latter. Either way, window.open
> being disabled on OS X while working on other platforms would be a pitfall
> that I think we should avoid. To get platform parity, I'd prefer exiting DOM
> fullscreen mode and opening the popup.

I proposed for both fullscreen mode, DOM one & normal (manual) one.

In the first place, *on OS X*, opening the popup as a "new window" from fulscreen-ed window. But this behavior is very annoying because opening new window from fulscreen-ed window creates a new another workspace (this is Firefox's current behavior).
This effect is made in both fullscreen, DOM one & normal one. This is very annoying because it will be expected that the popup share same workspace. (I wrote similar things at comment #29.)

So my proposal is that this patch fixes such problem on OSX only and makes the  loophole pref for other platforms. The latter is aiming to fix the problem of app's fullscreen mode on all desktop gui platforms which I wrote above a number of times. But this change on non-OSX platform may cause user confuse. Thus we provide the pref which is false by default.

Well, the behavior gap between platforms might be bad. But it is only specified in any standard spec. Other non-specified behaviors should follow the platform which the browser run on, that I think.

If DOM fullscreen spec has already decided that opening the popup in DOM fullscreen mode should be block or should open to a new window, of course I think we must follow it.
(In reply to Tetsuharu OHZEKI [:saneyuki_s] from comment #65)
> Well, the behavior gap between platforms might be bad.

It is. It means that people's work developed on one platform suddenly stops working when users are on a different platform.

> But it is only
> specified in any standard spec. Other non-specified behaviors should follow
> the platform which the browser run on, that I think.

I'm not sure what you're trying to say here. What I proposed (exiting DOM FS and opening the popup) does follow the platform.

> If DOM fullscreen spec has already decided that opening the popup in DOM
> fullscreen mode should be block or should open to a new window, of course I
> think we must follow it.

If the spec doesn't say anything about it (which I suppose is the case), it means that things should just work™.
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #64)
> Dao:
> 
> There are three fullscreen modes. One is the DOM fullscreen mode which is
> caused by website script. Next is user specified fullscreen mode with F11
> key. The last is OS X native fullscreen mode.
> 
> I think that the original suggestion of this bug is for the last one. But I
> think that in the second case should behave same as the last case since both
> cases cannot change windows by pointing devices.

Yes. just like so.
(In reply to Tetsuharu OHZEKI [:saneyuki_s] from comment #67)
> (In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #64)
> > There are three fullscreen modes. One is the DOM fullscreen mode which is
> > caused by website script. Next is user specified fullscreen mode with F11
> > key. The last is OS X native fullscreen mode.
> > 
> > I think that the original suggestion of this bug is for the last one. But I
> > think that in the second case should behave same as the last case since both
> > cases cannot change windows by pointing devices.
> 
> Yes. just like so.

I know that's your thinking, but other browsers, dolske and I disagree with you here. We've yet to see what UX people have to say.
(In reply to Dão Gottwald [:dao] from comment #68)
> (In reply to Tetsuharu OHZEKI [:saneyuki_s] from comment #67)
> > (In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #64)
> > > There are three fullscreen modes. One is the DOM fullscreen mode which is
> > > caused by website script. Next is user specified fullscreen mode with F11
> > > key. The last is OS X native fullscreen mode.
> > > 
> > > I think that the original suggestion of this bug is for the last one. But I
> > > think that in the second case should behave same as the last case since both
> > > cases cannot change windows by pointing devices.
> > 
> > Yes. just like so.
> 
> I know that's your thinking, but other browsers, dolske and I disagree with
> you here. We've yet to see what UX people have to say.

I'm with Dao. I think this change should only be made for Lion's fullscreen.
This behavior change is only on OSX by the pref because ifdef preprocesssing makes codes into complex.
Attachment #692289 - Attachment is obsolete: true
Attachment #692289 - Flags: ui-review?(madhava)
Attachment #708387 - Flags: review?(dolske)
Comment on attachment 708387 [details] [diff] [review]
part-1 v3: window.open from content should open a when browser is fullscreen.

In the DOM fullscreen case, this patch will still cause Firefox to both exit fullscreen mode and needlessly redirect the popup to a tab, won't it?
(In reply to Dão Gottwald [:dao] from comment #71)
> Comment on attachment 708387 [details] [diff] [review]
> part-1 v3: window.open from content should open a when browser is fullscreen.
> 
> In the DOM fullscreen case, this patch will still cause Firefox to both exit
> fullscreen mode and needlessly redirect the popup to a tab, won't it?

Yes.

I think that this is not fully intelligent. So it should behave this (like Chromium on OSX):
STEP:
1. Firefox is in the browser's fullscreen mode (This is normaly apps fullscreen mode).
2. And Firefox enters the DOM fullscreen mode.
3. Firefox opens the popup.
4. Exit the DOM fullscreen mode, but keep in the browser's fullscreen mode.

However, I think also that the current patches doesn't have some fatal UX problem. And 18 months has elapsed since OSX Lion has released. We need to resolve this bug as soon as possible.
So I have not added the above intelligent behavior to these patches yet. We should do it on the new bug after resolved this.
(In reply to Tetsuharu OHZEKI [:saneyuki_s] from comment #70)
> Created attachment 708387 [details] [diff] [review]
> part-1 v3: window.open from content should open a when browser is fullscreen.
> 
> This behavior change is only on OSX by the pref because ifdef preprocesssing
> makes codes into complex.

This change should be made only for Lion's native fullscreen mode.
On an older (i.e. 10.6) OSX, this should stay in the old fashion.
(In reply to mnehmiz from comment #73)
> This change should be made only for Lion's native fullscreen mode.
> On an older (i.e. 10.6) OSX, this should stay in the old fashion.

There is some sense in your point.
However, it would be needless complex if we add to check OSX's version & consider the behavior.
And, I don't think that Firefox need to change the behavior to the old fashion. At almost GUI desktop environment, user cannot focus the window which is behind the front window. This causes the annoying problem. If browser showed the new popup window, and user changed the focus from its popup after it opened, User cannot refocus the opened popup window without exiting the fullscreen mode. This problem is very annoying. But we can resolve it by opening the popup into tabs when browser is in fullscreen mode.
(In reply to Tetsuharu OHZEKI [:saneyuki_s] from comment #74)
> [...] This problem is very annoying.
If you don't use a keyboard ...

As long as there's an about:config option for it, I'm fine with that. But I really like my GUI old-fashioned. That's why I'm still on 10.6 and switching windows with the keyboard.
(In reply to mnehmiz from comment #75)
> (In reply to Tetsuharu OHZEKI [:saneyuki_s] from comment #74)
> > [...] This problem is very annoying.
> If you don't use a keyboard ...
> 
> As long as there's an about:config option for it, I'm fine with that. But I
> really like my GUI old-fashioned. That's why I'm still on 10.6 and switching
> windows with the keyboard.

Keyboard shortcut is the one of approaches. But its operation does not complete in pointing device. (This may be a liking,) I think it's not good.

By the way, if you hope it on current patches, you can use old-fashion with setting the pref |browser.link.open_newwindow.disabled_in_fullscreen| is |false|.
Attachment #708387 - Flags: review?(dolske) → review?(bzbarsky)
Attachment #692550 - Flags: review?(bzbarsky)
Comment on attachment 692550 [details] [diff] [review]
part-0 v2: Reduce to check the caller context

r=me
Attachment #692550 - Flags: review?(bzbarsky) → review+
Comment on attachment 708387 [details] [diff] [review]
part-1 v3: window.open from content should open a when browser is fullscreen.

r=me, I guess, though see comments about Lion vs earlier....
Attachment #708387 - Flags: review?(bzbarsky) → review+
(In reply to Boris Zbarsky (:bz) from comment #78)
> r=me, I guess, though see comments about Lion vs earlier....

Umm, it's a difficult problem.
Basically, my stance is that browser should open the popup in new tabs when browser is in application fullscreen mode.
And can we get the OSX's version with low cost?

I think, the other approach is that adding the visible preference menu to switch this behavior to Preference panel. But I feel this is impolicy!
I adjusted the pref file comment, since it was rather confusing as-is:
https://hg.mozilla.org/integration/mozilla-inbound/rev/cb90cb8da5e2
(In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #82)
> I adjusted the pref file comment, since it was rather confusing as-is:
> https://hg.mozilla.org/integration/mozilla-inbound/rev/cb90cb8da5e2

Thank you for catching, Gavin.
You need to log in before you can comment on or make changes to this bug.