New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 832765 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

immersive header doesn't reveal on chrome apps once deactivated

Project Member Reported by osh...@chromium.org, Apr 13 2018

Issue description

chrome version: ToT, 66.0.3559.79

Repo step:
1) start files app
2) go fullscreen. you can reveal the frame
3) open chrome (or activate another chrome window)
4) switch to files app

expected:
you can reveal the header by moving moues to the top

acutal:
you can't.

looks like apps disable immersive mode upon deactivation, but doesn't
re-enable upon activation.

It's probably too late for 66,but we should fix this for 67.
 

Comment 1 by osh...@chromium.org, Apr 13 2018

Cc: sammiequon@chromium.org
Owner: osh...@chromium.org
Status: Started (was: Untriaged)
this is due to the tablet mode immmersive change. will send a fix soon.
Project Member

Comment 2 by bugdroid1@chromium.org, Apr 16 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2aaf69457df67f37ea7560da221b5228f152b396

commit 2aaf69457df67f37ea7560da221b5228f152b396
Author: Mitsuru Oshima <oshima@chromium.org>
Date: Mon Apr 16 18:51:56 2018

Make sure immersive state is always correct

added UpdateImmersiveMode() which updates the immersive mode
with the latest status and use it everywhere.

This fixes the issue with activation.

BUG= 832765 
TEST=covered by unit test.

Change-Id: If1842e523d601dfba40973f5bf62459a7f7f86a6
Reviewed-on: https://chromium-review.googlesource.com/1012993
Commit-Queue: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: Sammie Quon <sammiequon@chromium.org>
Reviewed-by: Ben Wells <benwells@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551056}
[modify] https://crrev.com/2aaf69457df67f37ea7560da221b5228f152b396/chrome/browser/ui/views/apps/chrome_native_app_window_views_aura_ash.cc
[modify] https://crrev.com/2aaf69457df67f37ea7560da221b5228f152b396/chrome/browser/ui/views/apps/chrome_native_app_window_views_aura_ash.h
[modify] https://crrev.com/2aaf69457df67f37ea7560da221b5228f152b396/chrome/browser/ui/views/apps/chrome_native_app_window_views_aura_ash_browsertest.cc
[modify] https://crrev.com/2aaf69457df67f37ea7560da221b5228f152b396/extensions/browser/app_window/app_window.cc
[modify] https://crrev.com/2aaf69457df67f37ea7560da221b5228f152b396/extensions/browser/app_window/app_window.h

Comment 3 by osh...@chromium.org, Apr 16 2018

Labels: Merge-Rejected-67

Comment 4 by osh...@chromium.org, Apr 16 2018

Labels: -Merge-Rejected-67 Merge-Request-67
Project Member

Comment 5 by bugdroid1@chromium.org, Apr 17 2018

Labels: merge-merged-testbranch
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2aaf69457df67f37ea7560da221b5228f152b396

commit 2aaf69457df67f37ea7560da221b5228f152b396
Author: Mitsuru Oshima <oshima@chromium.org>
Date: Mon Apr 16 18:51:56 2018

Make sure immersive state is always correct

added UpdateImmersiveMode() which updates the immersive mode
with the latest status and use it everywhere.

This fixes the issue with activation.

BUG= 832765 
TEST=covered by unit test.

Change-Id: If1842e523d601dfba40973f5bf62459a7f7f86a6
Reviewed-on: https://chromium-review.googlesource.com/1012993
Commit-Queue: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: Sammie Quon <sammiequon@chromium.org>
Reviewed-by: Ben Wells <benwells@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551056}
[modify] https://crrev.com/2aaf69457df67f37ea7560da221b5228f152b396/chrome/browser/ui/views/apps/chrome_native_app_window_views_aura_ash.cc
[modify] https://crrev.com/2aaf69457df67f37ea7560da221b5228f152b396/chrome/browser/ui/views/apps/chrome_native_app_window_views_aura_ash.h
[modify] https://crrev.com/2aaf69457df67f37ea7560da221b5228f152b396/chrome/browser/ui/views/apps/chrome_native_app_window_views_aura_ash_browsertest.cc
[modify] https://crrev.com/2aaf69457df67f37ea7560da221b5228f152b396/extensions/browser/app_window/app_window.cc
[modify] https://crrev.com/2aaf69457df67f37ea7560da221b5228f152b396/extensions/browser/app_window/app_window.h

This isn't a M67 regression; reported for M66.  This is a fairly large change risking M67 stability.  Compelling reason to force it in?

Comment 7 by osh...@chromium.org, Apr 17 2018

It is true that this is regression introduced early this year. 
I don't think this is large compared to other CLs we merged in the past.

A user won't be able to access the frame once deactivated the fullscreen.
Workaround is that you can exit fullscreen using shortcut. (could be hard in tablet though)


I didn't request to merge 66 because I thought it's too late for 66.
I'll leave it to you though.
Project Member

Comment 8 by sheriffbot@chromium.org, Apr 18 2018

Labels: -Merge-Request-67 Merge-Approved-67 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M67. Please go ahead and merge the CL to branch 3396 manually. Please contact milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 9 by bugdroid1@chromium.org, Apr 18 2018

Labels: -merge-approved-67 merge-merged-3396
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7b4bee2a9166c330ecb1401a0617498df6008a63

commit 7b4bee2a9166c330ecb1401a0617498df6008a63
Author: Mitsuru Oshima <oshima@chromium.org>
Date: Wed Apr 18 20:07:34 2018

Make sure immersive state is always correct

added UpdateImmersiveMode() which updates the immersive mode
with the latest status and use it everywhere.

This fixes the issue with activation.

BUG= 832765 
TEST=covered by unit test.

Change-Id: If1842e523d601dfba40973f5bf62459a7f7f86a6
Reviewed-on: https://chromium-review.googlesource.com/1012993
Commit-Queue: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: Sammie Quon <sammiequon@chromium.org>
Reviewed-by: Ben Wells <benwells@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#551056}(cherry picked from commit 2aaf69457df67f37ea7560da221b5228f152b396)
Reviewed-on: https://chromium-review.googlesource.com/1017661
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#97}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/7b4bee2a9166c330ecb1401a0617498df6008a63/chrome/browser/ui/views/apps/chrome_native_app_window_views_aura_ash.cc
[modify] https://crrev.com/7b4bee2a9166c330ecb1401a0617498df6008a63/chrome/browser/ui/views/apps/chrome_native_app_window_views_aura_ash.h
[modify] https://crrev.com/7b4bee2a9166c330ecb1401a0617498df6008a63/chrome/browser/ui/views/apps/chrome_native_app_window_views_aura_ash_browsertest.cc
[modify] https://crrev.com/7b4bee2a9166c330ecb1401a0617498df6008a63/extensions/browser/app_window/app_window.cc
[modify] https://crrev.com/7b4bee2a9166c330ecb1401a0617498df6008a63/extensions/browser/app_window/app_window.h

Cc: josa...@chromium.org
Status: Fixed (was: Started)
+josafat@ who is m66 owner.

I'm closing this because we're very close to m66 stable release. Please reopen if you think we should merge this.

Sign in to add a comment