Exit of fullscreen view when a tab is closed

[romuloalves]

Describe the issue you encountered: When I am with Brave in fullscreen view and close a tab, Brave exits of fullscreen.

Expected behavior: Just close the tab and don't exit of fullscreen view.

• Platform (Win7, 8, 10? macOS? Linux distro?): macOS
• Brave Version: 0.11.1
• Steps to reproduce:
  1. Enter in fullscreen view
  2. Close a tab

[bbondy]

We had the opposite request in the past. Can you describe a use case? Do any other browsers do this?

[romuloalves]

I don't know if I expressed myself correctly.
I think what is happening is a problem.

I'm with Brave open in fullscreen view:

I just close a tab:

And Brave exits from fullscreen view by itself:

No other browser has this behavior.

[molinadavid]

I can confirm the same and it should be a bug, it was introduced on version 0.11.1

Brave: 0.11.1
Electron: 1.2.7
libchromiumcontent: 51.0.2704.103
V8: 5.1.281.65
Node.js: 6.1.0
Update Channel: dev

whenever you have the browser on full screen mode (distraction free) "ctrl+command+f" and close a tab from the browser the browser itself will trigger the event to go out of full screen mode and back into normal desktop mode.

The opposite is not true, if I close a tab while on desktop mode it will not enter full screen mode.

[benmosher]

from #2404:

Expected behavior:
I expect Brave to revert into non fullscreen mode or whatever mode I had before.

(emphasis mine)

I think the point of this new issue that closing a tab should not exit fullscreen mode unless the current tab (only) promoted the window to fullscreen. And it's not clear to me how you could even know that.

I think 8cded9c should be reverted until it can be made more targeted to apply only to #2404.

I use Brave in fullscreen (OSX, split screen with two windows) exclusively, and now I can't ever close tabs without having to drag it back to a fullscreen space. 😓

[benmosher]

BTW, I can confirm that at least Opera doesn't always leave fullscreen if any tab is closed.

[bbondy]

I think maybe html5 (e.g. video) fullscreen should exit full screen and full screen mode at the browser level should not.
Can someone check with how chrome works with the 2 different types of fullscreen?
See #2404

[benmosher]

Chrome exits fullscreen if it was prompted by YouTube, but not in normal fullscreen.

Seems like ctrl-w should close the tab + trigger the same event asesc when in a full-screen video. That would match Chrome and satisfy both this issue and #2404.

[bbondy]

that sounds best, thanks

[jikkujose]

I am also confirming the issue. Simply closing a tab takes the browser off from full screen mode.

[bbondy]

closing since this is fixed and will be in the next version.

[luixxiul]

Is this only for macOS? I cannot confirm this was fixed for Windows 7

#2768

[srirambv]

Issue confirmed on 0.11.2 Beta1 for Windows x64 install. Closing of any tab exits full screen and back to restore size.

[benmosher]

It appears my commits are not present in the tag for v0.11.2dev-beta1: https://github.com/brave/browser-laptop/compare/v0.11.2dev-beta1...benmosher:keep-fullscreen?expand=1

I can't speak for whether they will fix Windows as well, but given the little familiarity I have now with the code, I think odds are good.

[benmosher]

Actually, the commits/changes don't appear to be present on dev-channel anymore, either?

[benmosher]

So my PR was against the dev-channel branch (#2631), was that wrong? did it get reset to master at some point?

I can rebase against wherever and resubmit if needed, commits are on benmosher:keep-fullscreen.

[luixxiul]

I think it's merged to master, @bbondy ?

[benmosher]

Changes don't appear to be present on master, just looking in the files. Also, commits aren't there either.

[bbondy]

Oh I didn't notice that @benmosher, I'll try to watch that in case a contributor does that again.
dev-channel is not meant for doing PRs to, it gets reset to master every release.

[bbondy]

I'll pull it in now.

[bbondy]

into master+dev-channel and it'll be in the next respin.

[bbondy]

90dcd8a
af7ffb1

[benmosher]

Ah, sorry about that!

When I based the first PR on master after forking from the v0.11.1dev tag, there were some extra commits on dev-channel that weren't on master. Will remember for next time. 😅

[bbondy]

np thanks for noticing!

[kingscott]

I noticed this today as well. I closed my issue, but I was able to repro.

[alexwykoff]

Looks good!