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

Issue 690295 link

Starred by 2 users
Status: Archived
Owner:
█ warx@chromium.org
Last visit > 30 days ago
Closed: Feb 2017
Cc:
█ keta...@chromium.org
pkasting@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug-Regression



Sign in to add a comment

ChromeOS tab-strip auto-hide does not work correctly with the bookmarks bar

Project Member Reported by w...@chromium.org, Feb 9 2017 
Chrome Version: 57.0.2987.32 dev
OS: ChromeOS

What steps will reproduce the problem?
(1) Configure some bookmarks.
(2) Open a tab to New Tab Page, so the bookmarks bar is shown.
(3) Press the ChromeOS full-screen button, and let the tab-strip auto-hide.
(4) Click on a bookmark.

What is the expected result?

Chosen bookmark should be opened in the tab.

What happens instead?

The window behaves as though the tab-strip were still visible. e.g. if the user clicked a bookmark that happened to be where the new-tab button was when the strip was visible, then a new tab will be opened.

Comment 1 by keta...@chromium.org, Feb 9 2017

Labels: -ReleaseBlock-Beta
From the severity this doesn't look like a beta blocker.

Comment 2 by w...@chromium.org, Feb 9 2017

Labels: ReleaseBlock-Stable
Punting from RB-Beta is fine, but we definitely don't want to be shipping known-broken UI to Stable.

Comment 3 by sky@chromium.org, Feb 9 2017

Owner: warx@chromium.org
Status: Assigned (was: Untriaged)
warx, I suspect this is because of the recent change to make the tabstrip visible in immersive-md.

Comment 4 by warx@chromium.org, Feb 9 2017 

Yeah, seen from "what happens instead", my cl should be the culprit. Will take a look.

Comment 5 by tdander...@chromium.org, Feb 14 2017

Labels: Proj-MaterialDesign-CrOS
Project Member

Comment 6 by bugdroid1@chromium.org, Feb 16 2017 

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

commit f05f264d80c72ec2681cbd4b3854ab6ceceaee67
Author: warx <warx@chromium.org>
Date: Thu Feb 16 18:43:20 2017

cros-md: Remove the non-MD immersive mode code paths

Changes:
Remove the non-MD immersive mode code paths

BUG= 690295 
BUG= 685831 
TEST=emulator test, automation tests, also add test coverage

Review-Url: https://codereview.chromium.org/2690443002
Cr-Commit-Position: refs/heads/master@{#451022}

[modify] https://crrev.com/f05f264d80c72ec2681cbd4b3854ab6ceceaee67/ash/common/material_design/material_design_controller.cc
[modify] https://crrev.com/f05f264d80c72ec2681cbd4b3854ab6ceceaee67/ash/common/material_design/material_design_controller.h
[modify] https://crrev.com/f05f264d80c72ec2681cbd4b3854ab6ceceaee67/ash/common/shelf/shelf_layout_manager.cc
[modify] https://crrev.com/f05f264d80c72ec2681cbd4b3854ab6ceceaee67/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc
[modify] https://crrev.com/f05f264d80c72ec2681cbd4b3854ab6ceceaee67/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.h
[modify] https://crrev.com/f05f264d80c72ec2681cbd4b3854ab6ceceaee67/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash_browsertest.cc
[modify] https://crrev.com/f05f264d80c72ec2681cbd4b3854ab6ceceaee67/chrome/browser/ui/views/frame/browser_non_client_frame_view_mus.cc
[modify] https://crrev.com/f05f264d80c72ec2681cbd4b3854ab6ceceaee67/chrome/browser/ui/views/frame/browser_non_client_frame_view_mus.h
[modify] https://crrev.com/f05f264d80c72ec2681cbd4b3854ab6ceceaee67/chrome/browser/ui/views/frame/browser_view_layout.cc
[modify] https://crrev.com/f05f264d80c72ec2681cbd4b3854ab6ceceaee67/chrome/browser/ui/views/frame/browser_view_layout_unittest.cc
[modify] https://crrev.com/f05f264d80c72ec2681cbd4b3854ab6ceceaee67/chrome/browser/ui/views/frame/immersive_mode_controller.h
[modify] https://crrev.com/f05f264d80c72ec2681cbd4b3854ab6ceceaee67/chrome/browser/ui/views/frame/immersive_mode_controller_ash.cc
[modify] https://crrev.com/f05f264d80c72ec2681cbd4b3854ab6ceceaee67/chrome/browser/ui/views/frame/immersive_mode_controller_ash.h
[modify] https://crrev.com/f05f264d80c72ec2681cbd4b3854ab6ceceaee67/chrome/browser/ui/views/frame/immersive_mode_controller_ash_unittest.cc
[modify] https://crrev.com/f05f264d80c72ec2681cbd4b3854ab6ceceaee67/chrome/browser/ui/views/frame/immersive_mode_controller_stub.cc
[modify] https://crrev.com/f05f264d80c72ec2681cbd4b3854ab6ceceaee67/chrome/browser/ui/views/frame/immersive_mode_controller_stub.h
[modify] https://crrev.com/f05f264d80c72ec2681cbd4b3854ab6ceceaee67/chrome/browser/ui/views/tabs/tab.cc
[modify] https://crrev.com/f05f264d80c72ec2681cbd4b3854ab6ceceaee67/chrome/browser/ui/views/tabs/tab.h
[modify] https://crrev.com/f05f264d80c72ec2681cbd4b3854ab6ceceaee67/chrome/browser/ui/views/tabs/tab_controller.h
[modify] https://crrev.com/f05f264d80c72ec2681cbd4b3854ab6ceceaee67/chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc
[modify] https://crrev.com/f05f264d80c72ec2681cbd4b3854ab6ceceaee67/chrome/browser/ui/views/tabs/tab_strip.cc
[modify] https://crrev.com/f05f264d80c72ec2681cbd4b3854ab6ceceaee67/chrome/browser/ui/views/tabs/tab_strip.h
[modify] https://crrev.com/f05f264d80c72ec2681cbd4b3854ab6ceceaee67/chrome/browser/ui/views/tabs/tab_strip_unittest.cc
[modify] https://crrev.com/f05f264d80c72ec2681cbd4b3854ab6ceceaee67/chrome/browser/ui/views/tabs/tab_unittest.cc

Comment 7 by warx@chromium.org, Feb 16 2017

Cc: keta...@chromium.org
Labels: Merge-Request-57
Merge request to M57 for the above CL. It looks like a big CL, but most of them are mechanical cleanup. Besides, md-immersive was on m55. So it should be safe to be merged to M57.

Comment 8 by keta...@chromium.org, Feb 16 2017

Labels: -Merge-Request-57 Merge-Approved-57
Approving merge to M57 Chrome OS.
Project Member

Comment 9 by bugdroid1@chromium.org, Feb 17 2017

Labels: -merge-approved-57 merge-merged-2987
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9a775b78d6cc7185af83e1095d6919505b3b7cba

commit 9a775b78d6cc7185af83e1095d6919505b3b7cba
Author: Qiang Xu <warx@chromium.org>
Date: Fri Feb 17 00:31:17 2017

[Merge to M57] cros-md: Remove the non-MD immersive mode code paths

Changes:
Remove the non-MD immersive mode code paths

TBR=tdanderson@chromium.org, sky@chromium.org
BUG= 690295 
BUG= 685831 
TEST=emulator test, automation tests, also add test coverage

Review-Url: https://codereview.chromium.org/2690443002
Cr-Commit-Position: refs/heads/master@{#451022}
(cherry picked from commit f05f264d80c72ec2681cbd4b3854ab6ceceaee67)

Review-Url: https://codereview.chromium.org/2702663002 .
Cr-Commit-Position: refs/branch-heads/2987@{#567}
Cr-Branched-From: ad51088c0e8776e8dcd963dbe752c4035ba6dab6-refs/heads/master@{#444943}

[modify] https://crrev.com/9a775b78d6cc7185af83e1095d6919505b3b7cba/ash/common/material_design/material_design_controller.cc
[modify] https://crrev.com/9a775b78d6cc7185af83e1095d6919505b3b7cba/ash/common/material_design/material_design_controller.h
[modify] https://crrev.com/9a775b78d6cc7185af83e1095d6919505b3b7cba/ash/common/shelf/shelf_layout_manager.cc
[modify] https://crrev.com/9a775b78d6cc7185af83e1095d6919505b3b7cba/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc
[modify] https://crrev.com/9a775b78d6cc7185af83e1095d6919505b3b7cba/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.h
[modify] https://crrev.com/9a775b78d6cc7185af83e1095d6919505b3b7cba/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash_browsertest.cc
[modify] https://crrev.com/9a775b78d6cc7185af83e1095d6919505b3b7cba/chrome/browser/ui/views/frame/browser_non_client_frame_view_mus.cc
[modify] https://crrev.com/9a775b78d6cc7185af83e1095d6919505b3b7cba/chrome/browser/ui/views/frame/browser_non_client_frame_view_mus.h
[modify] https://crrev.com/9a775b78d6cc7185af83e1095d6919505b3b7cba/chrome/browser/ui/views/frame/browser_view_layout.cc
[modify] https://crrev.com/9a775b78d6cc7185af83e1095d6919505b3b7cba/chrome/browser/ui/views/frame/browser_view_layout_unittest.cc
[modify] https://crrev.com/9a775b78d6cc7185af83e1095d6919505b3b7cba/chrome/browser/ui/views/frame/immersive_mode_controller.h
[modify] https://crrev.com/9a775b78d6cc7185af83e1095d6919505b3b7cba/chrome/browser/ui/views/frame/immersive_mode_controller_ash.cc
[modify] https://crrev.com/9a775b78d6cc7185af83e1095d6919505b3b7cba/chrome/browser/ui/views/frame/immersive_mode_controller_ash.h
[modify] https://crrev.com/9a775b78d6cc7185af83e1095d6919505b3b7cba/chrome/browser/ui/views/frame/immersive_mode_controller_ash_unittest.cc
[modify] https://crrev.com/9a775b78d6cc7185af83e1095d6919505b3b7cba/chrome/browser/ui/views/frame/immersive_mode_controller_stub.cc
[modify] https://crrev.com/9a775b78d6cc7185af83e1095d6919505b3b7cba/chrome/browser/ui/views/frame/immersive_mode_controller_stub.h
[modify] https://crrev.com/9a775b78d6cc7185af83e1095d6919505b3b7cba/chrome/browser/ui/views/tabs/tab.cc
[modify] https://crrev.com/9a775b78d6cc7185af83e1095d6919505b3b7cba/chrome/browser/ui/views/tabs/tab.h
[modify] https://crrev.com/9a775b78d6cc7185af83e1095d6919505b3b7cba/chrome/browser/ui/views/tabs/tab_controller.h
[modify] https://crrev.com/9a775b78d6cc7185af83e1095d6919505b3b7cba/chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc
[modify] https://crrev.com/9a775b78d6cc7185af83e1095d6919505b3b7cba/chrome/browser/ui/views/tabs/tab_strip.cc
[modify] https://crrev.com/9a775b78d6cc7185af83e1095d6919505b3b7cba/chrome/browser/ui/views/tabs/tab_strip.h
[modify] https://crrev.com/9a775b78d6cc7185af83e1095d6919505b3b7cba/chrome/browser/ui/views/tabs/tab_strip_unittest.cc
[modify] https://crrev.com/9a775b78d6cc7185af83e1095d6919505b3b7cba/chrome/browser/ui/views/tabs/tab_unittest.cc

Comment 10 by warx@chromium.org, Feb 17 2017

Status: Fixed (was: Assigned)

Comment 11 by dchan@chromium.org, Aug 1 2017

Labels: VerifyIn-61

Comment 12 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)
►

Sign in to add a comment