[desktop-pwas] Hosted app windows caption buttons height is not locked to the title bar height |
||||
Issue descriptionChrome Version: 67.0.3383.0 OS: Chrome What steps will reproduce the problem? (1) Start a hosted app window (or bookmark app, or PWA). (2) Maximize it. (3) Mouse to top row of pixels. What is the expected result? Caption buttons (min/max/close) should be clickable. What happens instead? Caption buttons (min/max/close) are not clickable on the top row of pixels, violating Fitts' Law. I think this is a recent regression. Noticed on Chromebook Pixel 2013 (maybe unique to this?)
,
Apr 13 2018
Not reproducible on Chromebook Pixel 2015, 67.0.3376.0. (Updating now.)
,
Apr 13 2018
Reproduces on Chromebook Pixel 2015, 67.0.3383.0. Regression in range 67.0.3376.0 -- 67.0.3383.0. According to alancutter, not reproducible on a ToT build (r550407).
,
Apr 13 2018
67.0.3383.0 was from Wed Mar 28 20:08:02 2018 -0700 at refs/heads/master@{#546671}.
,
Apr 13 2018
Screenshot from r550057. Measurements: In ToT: both title bar and button are 33 px tall. In 67.0.3383.0: title bar is 72 px tall. Caption button is 66 px tall. May actually be Issue 830285 that was already fixed (we don't have the fix on any of these real devices).
,
Apr 13 2018
I wasn't able to repro the bug in 67.0.3383.0 using a Linux desktop chromeOS build so my device isn't a good indicator of it being fixed.
,
Apr 13 2018
Reproducible on a Pixelbook at the same version (67.0.3383.0). Not reproducible from a local build at the same version (r546671). (So that's inconsistent.) But... NOT reproducible on a Pixelbook at Canary (r548636). So I think this was transient and got fixed. Still concerned that this code is brittle --- if the height of the title bar ever changes, the caption buttons will remain small and break Fitts' Law again. I'll keep this bug open and bump the priority/milestone since I think this can be refactored to have the caption buttons' height extend all the way no matter what.
,
Apr 13 2018
As we've seen with issue 830285 the titlebar height is susceptible to changing based on its child views. I'm not sure if a dynamic height titlebar is every desirable and perhaps it should be a fixed height that the child views adhere to.
,
Apr 16 2018
#9: A fixed height title bar pushes the problem elsewhere. Child views that resize will get clipped, possibly with nobody noticing. Adding a test that enforces the height may help with this (I think one was added to the BrowserActionsContainer fix). Extending the buttons to the top also sgtm, but may make a transition to BoxLayout in the title bar harder since it doesn't support per-view cross axis alignments. We could also just add that.
,
Apr 24 2018
Created a CL that adds assertions that the close button occupies the top right corner: https://chromium-review.googlesource.com/c/chromium/src/+/1025493
,
Apr 24 2018
,
Apr 30 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/78a019ab64ece8d95610f678d3aa36f63675fb4d commit 78a019ab64ece8d95610f678d3aa36f63675fb4d Author: Alan Cutter <alancutter@chromium.org> Date: Mon Apr 30 05:59:41 2018 Assert the top right corner of the Ash title bar is occupied by the close button This CL provides guarantees that the close button is in the top right of each window frame. This change fixes a couple of bugs where transitionary layouts didn't meet this assertion. Bug: 832338 Change-Id: I44cc946b80fac8244f4f9b66f2738df837be552d Reviewed-on: https://chromium-review.googlesource.com/1025493 Commit-Queue: Alan Cutter <alancutter@chromium.org> Reviewed-by: Michael Wasserman <msw@chromium.org> Reviewed-by: Mitsuru Oshima (OOO 4/30 - 5/9) <oshima@chromium.org> Cr-Commit-Position: refs/heads/master@{#554699} [modify] https://crrev.com/78a019ab64ece8d95610f678d3aa36f63675fb4d/ash/frame/caption_buttons/frame_caption_button_container_view.cc [modify] https://crrev.com/78a019ab64ece8d95610f678d3aa36f63675fb4d/ash/frame/caption_buttons/frame_caption_button_container_view.h [modify] https://crrev.com/78a019ab64ece8d95610f678d3aa36f63675fb4d/ash/frame/caption_buttons/frame_caption_button_container_view_unittest.cc [modify] https://crrev.com/78a019ab64ece8d95610f678d3aa36f63675fb4d/chrome/browser/ui/views/frame/browser_frame_header_ash.cc [modify] https://crrev.com/78a019ab64ece8d95610f678d3aa36f63675fb4d/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc
,
Apr 30 2018
|
||||
►
Sign in to add a comment |
||||
Comment 1 by mgiuca@chromium.org
, Apr 12 2018