New issue
Advanced search Search tips

Issue 877723 link

Starred by 2 users

Issue metadata

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


Participants' hotlists:
Gesture-polish


Sign in to add a comment

Settings app still has the window title bar that's not supposed to be there

Project Member Reported by kejiashao@google.com, Aug 25

Issue description

Chrome Version: (copy from chrome://version)
OS: (e.g. Win10, MacOS 10.12, etc...)

What steps will reproduce the problem?
(1) In settings app, swipe down from the top
(2) 
(3)

What is the expected result?
User can start dragging the app

What happens instead?
Window title bar drops down


Please use labels and text to provide additional information.

If this is a regression (i.e., worked before), please consider using the
bisect tool (https://www.chromium.org/developers/bisect-builds-py) to help
us identify the root cause and more rapidly triage the issue.

For graphics-related bugs, please copy/paste the contents of the about:gpu
page at the end of this report.


 
Cc: kejiashao@chromium.org
Cc: mkarkada@chromium.org dhadd...@chromium.org minch@chromium.org abod...@chromium.org
 Issue 889656  has been merged into this issue.
Status: Started (was: Assigned)
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 11

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

commit f4556fd0e59ab0f4e258149d832db0d3d3d118ca
Author: Min Chen <minch@google.com>
Date: Thu Oct 11 20:04:59 2018

Add fullscreen restriction for browser window drag.

This will partly fix the problem of Settings and Discover internal apps.
Both of their kAppType are set to BROWSER, which is not what we want for
window drag. For real browser window, its frame will only be hidden when
it is full-screened. Add the restriction here to avoid the problem of
Settings and Discover when they are only maximized.

Bug:  877723 
Change-Id: Ib6b6708b8065940a1a94be89cdde6b317093cdb2
Reviewed-on: https://chromium-review.googlesource.com/c/1274839
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Min Chen <minch@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598906}
[modify] https://crrev.com/f4556fd0e59ab0f4e258149d832db0d3d3d118ca/ash/wm/immersive_gesture_handler_classic.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Oct 12

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

commit cb343f3c5acf70010afafa8c498ff5104a533493
Author: Min Chen <minch@google.com>
Date: Fri Oct 12 16:27:16 2018

Set kAppType of Settings and Discover apps to CHROME_APP.

The kAppType of browser based custom apps are set based on the
|app_name|, which is not set for the internal apps like Settings and
Discover. Rewrite it for these two apps in corresponding window manager
to make sure both of them can have the correct app type now.

calamity@ is doing a refactoring of the system apps here
(https://docs.google.com/document/d/16CRTWFjU8Ohrn9aSTPuDJcbqh1z7E4wKwyEWstInRh0/edit#),
which will fix all these related issues in long term.

Bug:  877723 
Change-Id: Iaaf9fb4998a1416bae4a8c6fa5582d1ff2235326
Reviewed-on: https://chromium-review.googlesource.com/c/1258090
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Reviewed-by: calamity <calamity@chromium.org>
Commit-Queue: Min Chen <minch@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599245}
[modify] https://crrev.com/cb343f3c5acf70010afafa8c498ff5104a533493/chrome/browser/ui/settings_window_manager_chromeos.cc
[modify] https://crrev.com/cb343f3c5acf70010afafa8c498ff5104a533493/chrome/browser/ui/webui/chromeos/login/discover/discover_window_manager.cc

Labels: -M-70 Merge-Request-71 M-71
The second change was landed in M72, add Merge-Request-71 for it.
Project Member

Comment 7 by sheriffbot@chromium.org, Oct 16

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

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

Comment 8 by bugdroid1@chromium.org, Oct 17

Labels: -merge-approved-71 merge-merged-3578
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bb07678b22511f7e06a7f44f7a7d1ef6d157f2ff

commit bb07678b22511f7e06a7f44f7a7d1ef6d157f2ff
Author: Min Chen <minch@google.com>
Date: Wed Oct 17 00:44:40 2018

[Merge to M71]Set kAppType of Settings and Discover apps to CHROME_APP.

The kAppType of browser based custom apps are set based on the
|app_name|, which is not set for the internal apps like Settings and
Discover. Rewrite it for these two apps in corresponding window manager
to make sure both of them can have the correct app type now.

calamity@ is doing a refactoring of the system apps here
(https://docs.google.com/document/d/16CRTWFjU8Ohrn9aSTPuDJcbqh1z7E4wKwyEWstInRh0/edit#),
which will fix all these related issues in long term.

TBR=minch@chromium.org

(cherry picked from commit cb343f3c5acf70010afafa8c498ff5104a533493)

Bug:  877723 
Change-Id: Iaaf9fb4998a1416bae4a8c6fa5582d1ff2235326
Reviewed-on: https://chromium-review.googlesource.com/c/1258090
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Reviewed-by: calamity <calamity@chromium.org>
Commit-Queue: Min Chen <minch@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#599245}
Reviewed-on: https://chromium-review.googlesource.com/c/1285694
Reviewed-by: Min Chen <minch@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#80}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
[modify] https://crrev.com/bb07678b22511f7e06a7f44f7a7d1ef6d157f2ff/chrome/browser/ui/settings_window_manager_chromeos.cc
[modify] https://crrev.com/bb07678b22511f7e06a7f44f7a7d1ef6d157f2ff/chrome/browser/ui/webui/chromeos/login/discover/discover_window_manager.cc

Status: Fixed (was: Started)
Labels: Merge-Merged-71-3578
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/bb07678b22511f7e06a7f44f7a7d1ef6d157f2ff

Commit: bb07678b22511f7e06a7f44f7a7d1ef6d157f2ff
Author: minch@google.com
Commiter: minch@chromium.org
Date: 2018-10-17 00:44:40 +0000 UTC

[Merge to M71]Set kAppType of Settings and Discover apps to CHROME_APP.

The kAppType of browser based custom apps are set based on the
|app_name|, which is not set for the internal apps like Settings and
Discover. Rewrite it for these two apps in corresponding window manager
to make sure both of them can have the correct app type now.

calamity@ is doing a refactoring of the system apps here
(https://docs.google.com/document/d/16CRTWFjU8Ohrn9aSTPuDJcbqh1z7E4wKwyEWstInRh0/edit#),
which will fix all these related issues in long term.

TBR=minch@chromium.org

(cherry picked from commit cb343f3c5acf70010afafa8c498ff5104a533493)

Bug:  877723 
Change-Id: Iaaf9fb4998a1416bae4a8c6fa5582d1ff2235326
Reviewed-on: https://chromium-review.googlesource.com/c/1258090
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Reviewed-by: calamity <calamity@chromium.org>
Commit-Queue: Min Chen <minch@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#599245}
Reviewed-on: https://chromium-review.googlesource.com/c/1285694
Reviewed-by: Min Chen <minch@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#80}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}

Sign in to add a comment