Add back button to windowed websites |
|||||||||||||||
Issue descriptionFor web apps that open in a window and have a window bar, we should be adding a back button there as well for ARC++ window bar consistency. Also would be helpful for the Gmails, etc. of the world. Assigning to PM for triage. Edit [8/1/2017]: changed "Chrome apps" to "web apps"
,
Jul 27 2017
Not sure this is 61 but sure would be nice to be. Should be a small fix
,
Jul 27 2017
I really like this suggestion. We can target all gSuite apps to start with. We can triage with ENG and agree on timelines.
,
Jul 27 2017
I'm pushing back on pretty much EVERYTHING new for 61 at this point. Regardless of how small. :-) Does anyone know why the back button wasn't there in the first place. I'd like to understand the motivations for the current behavior before we make a "quick" change.
,
Jul 27 2017
Do we know the implication of back button on chrome apps? My guess is that most chrome apps does not expect back navigation happens. (and application itself can't listen back key)
,
Jul 27 2017
The back button would be browser back since it would be acting on a webpage.
,
Jul 27 2017
RE #4, the desire for this change is to make our app set more unified and usable — showing/not showing the back button is surfacing a seam to users that isn't easy to explain. It's going to be particularly noticeable with our new default app set, all of which will open as windowed to start.
,
Jul 27 2017
Totally understand the motivation for the change. I'm just trying to understand why it isn't that way already and what we're likely to break by doing so. This is the sort of thing I worry about not getting enough testing on. As oshima@ points out in #5 some apps might react poorly to the sudden and unexpected use of Back by their users. One hypothetical failure mode: A test taking app does not allow users to go back to previous questions but relies on the lack of a back button to enforce that behavior. Having back suddenly appear for their users would be BAD. It'd be stupid to design that way and I have no clue if anyone has done so (or any way to find out) but it's a class of error we'd expose with a change like this. I'm really reluctant to do this.
,
Jul 27 2017
Just had oshima@ & skuhne@ independently come to me in a panic over this. The word "impossible" was frequently used. Triggering an unexpected back navigation in an app that was never designed to handle it has effectively infinite failure possibilities. These aren't just web pages. If the app developer assumed back was impossible and it suddenly becomes possible literally ANYTHING could go wrong. The best we could do here would be to create some way to turn it on for our own apps and apps that were tested with it. The best option would be some new manifest key for an app to indicate that it's cool with the back button, but that's taking on a LOT of extra work that we don't have bandwidth for. Glad to discuss further face to face, but as described this isn't doable.
,
Jul 28 2017
Technically, it's possible to add back button to v1 app window, because v1 app is just a normal web page on the reduced browser UI. The back button already works on these windows. That's being said, adding to v1 app, but not v2 app would be confusing, so I guess we still want to avoid it. Only caveat is that tablet mode will have a back button, which will not work on chrome apps. +omti.
,
Jul 28 2017
Re #10, the back button in tablet mode actually works on Chrome apps. I tested a few of them and was pleasently surprised :) I think it's okay for now to keep this as WontFix and once we get more data on usage of back button in tablet mode we can decide whether it is worth adding it for Chrome apps beyond tablet mode.
,
Jul 28 2017
I can't image how it can work because it doesn't work in normal case. What app did you use?
,
Jul 28 2017
jennschen@, are you ok with Omri's proposal in Comment 10 - to wait and test back button usage in Tablets? Happy to explore this further.
,
Jul 28 2017
I just talked to Omri, and he misunderstood chrome app vs web app. So it still does not work on chrome apps.
,
Jul 31 2017
Wow, a lot of confusion in this thread 1) The request was for windows websites which expect back navigation naturally as they are just websites 2) We will see many more of those given this will be the default 3) Using the word "app" here was a bit of a misnomer, sorry about that. Nobody suggests doing changes for any Chrome apps I hope this clarifies this request
,
Aug 1 2017
,
Aug 1 2017
Thanks for the clarification. I have updated the description for the bug to reflect we are talking about web apps. In that case we expect web apps to accept back button and it makes sense.
,
Aug 1 2017
thanks for clarification! spoke with Oshima and this is very doable. Given how packed M62 is already, let's punt this to M63. cheers, Olga
,
Aug 3 2017
,
Aug 3 2017
to clarify - this is a request to add back button to streamlined hosted apps.
,
Aug 22 2017
,
Aug 29 2017
,
Oct 2 2017
,
Oct 2 2017
I'm adding back button for ARC++, so it shouldn't be hard to enable probably in 64 or 65 at least.
,
Oct 2 2017
,
Oct 2 2017
thanks Oshima - udpated to M65.
,
Oct 24 2017
Started as a part of ARC++ side caption work. The button will be behind the flag for v1 apps and we can lunch either in 64 or 65.
,
Oct 26 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/40338d68c94b7fbfdf27bfd071f9d1467bd9fe82 commit 40338d68c94b7fbfdf27bfd071f9d1467bd9fe82 Author: Mitsuru Oshima <oshima@chromium.org> Date: Thu Oct 26 20:00:42 2017 Add back button to frame. This will be used in two scenarios: 1) back button for v1 apps. This is currently behind --ash-enable-v1-back-button. This currently does not take the history into account and will be addressed in a separate CL. 2) back button for ARC++ apps. This is not hooked up yet and will be handled in a followup CL. When clicked, it generates back pressed/release key sequence. The icon data is just placeholder and will be updated with correct assets. BUG=749713, b/33693796 TEST=covered by unit tests. Also tested --mus and worked. Change-Id: I4925ade99337004a4bf3a6c6da53f5109ed68360 Reviewed-on: https://chromium-review.googlesource.com/699350 Reviewed-by: Scott Violet <sky@chromium.org> Reviewed-by: James Cook <jamescook@chromium.org> Commit-Queue: Mitsuru Oshima <oshima@chromium.org> Cr-Commit-Position: refs/heads/master@{#511935} [modify] https://crrev.com/40338d68c94b7fbfdf27bfd071f9d1467bd9fe82/ash/BUILD.gn [add] https://crrev.com/40338d68c94b7fbfdf27bfd071f9d1467bd9fe82/ash/frame/caption_buttons/frame_back_button.cc [add] https://crrev.com/40338d68c94b7fbfdf27bfd071f9d1467bd9fe82/ash/frame/caption_buttons/frame_back_button.h [modify] https://crrev.com/40338d68c94b7fbfdf27bfd071f9d1467bd9fe82/ash/frame/custom_frame_view_ash.cc [modify] https://crrev.com/40338d68c94b7fbfdf27bfd071f9d1467bd9fe82/ash/frame/custom_frame_view_ash.h [modify] https://crrev.com/40338d68c94b7fbfdf27bfd071f9d1467bd9fe82/ash/frame/custom_frame_view_ash_unittest.cc [modify] https://crrev.com/40338d68c94b7fbfdf27bfd071f9d1467bd9fe82/ash/frame/default_header_painter.cc [modify] https://crrev.com/40338d68c94b7fbfdf27bfd071f9d1467bd9fe82/ash/frame/default_header_painter.h [modify] https://crrev.com/40338d68c94b7fbfdf27bfd071f9d1467bd9fe82/ash/frame/default_header_painter_unittest.cc [modify] https://crrev.com/40338d68c94b7fbfdf27bfd071f9d1467bd9fe82/ash/frame/frame_border_hit_test.cc [modify] https://crrev.com/40338d68c94b7fbfdf27bfd071f9d1467bd9fe82/ash/frame/frame_border_hit_test.h [modify] https://crrev.com/40338d68c94b7fbfdf27bfd071f9d1467bd9fe82/ash/frame/header_painter.h [modify] https://crrev.com/40338d68c94b7fbfdf27bfd071f9d1467bd9fe82/ash/frame/header_view.cc [modify] https://crrev.com/40338d68c94b7fbfdf27bfd071f9d1467bd9fe82/ash/frame/header_view.h [modify] https://crrev.com/40338d68c94b7fbfdf27bfd071f9d1467bd9fe82/ash/public/cpp/ash_switches.cc [modify] https://crrev.com/40338d68c94b7fbfdf27bfd071f9d1467bd9fe82/ash/public/cpp/ash_switches.h [modify] https://crrev.com/40338d68c94b7fbfdf27bfd071f9d1467bd9fe82/ash/resources/vector_icons/BUILD.gn [add] https://crrev.com/40338d68c94b7fbfdf27bfd071f9d1467bd9fe82/ash/resources/vector_icons/window_control_back.1x.icon [add] https://crrev.com/40338d68c94b7fbfdf27bfd071f9d1467bd9fe82/ash/resources/vector_icons/window_control_back.icon [modify] https://crrev.com/40338d68c94b7fbfdf27bfd071f9d1467bd9fe82/ash/wm/panels/panel_frame_view.cc [modify] https://crrev.com/40338d68c94b7fbfdf27bfd071f9d1467bd9fe82/chrome/browser/ui/views/frame/browser_header_painter_ash.cc [modify] https://crrev.com/40338d68c94b7fbfdf27bfd071f9d1467bd9fe82/chrome/browser/ui/views/frame/browser_header_painter_ash.h [modify] https://crrev.com/40338d68c94b7fbfdf27bfd071f9d1467bd9fe82/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc [modify] https://crrev.com/40338d68c94b7fbfdf27bfd071f9d1467bd9fe82/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.h [modify] https://crrev.com/40338d68c94b7fbfdf27bfd071f9d1467bd9fe82/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash_browsertest.cc
,
Jan 9 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/78fc4448c09e227447778a59ccd736f8d482037d commit 78fc4448c09e227447778a59ccd736f8d482037d Author: Mitsuru Oshima <oshima@chromium.org> Date: Tue Jan 09 23:19:08 2018 Update backbutotn state Disable/enable the button based on navigation state. The appearance will be adjusted in a separate CL. BUG=749713 TEST=Covered by unit test. Change-Id: Id577c3971440bebfebb2490c7398870cd4f68c93 Reviewed-on: https://chromium-review.googlesource.com/843821 Reviewed-by: Scott Violet <sky@chromium.org> Reviewed-by: Mitsuru Oshima <oshima@chromium.org> Commit-Queue: Mitsuru Oshima <oshima@chromium.org> Cr-Commit-Position: refs/heads/master@{#528162} [modify] https://crrev.com/78fc4448c09e227447778a59ccd736f8d482037d/ash/frame/caption_buttons/frame_caption_button.cc [modify] https://crrev.com/78fc4448c09e227447778a59ccd736f8d482037d/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc [modify] https://crrev.com/78fc4448c09e227447778a59ccd736f8d482037d/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.h [modify] https://crrev.com/78fc4448c09e227447778a59ccd736f8d482037d/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash_browsertest.cc
,
Jan 10 2018
The functionality is in. What's left is polish UI, including. * correct asset for arrow. The current asset is just a placeholder I took it from arrow on shelf. * decide if we want to show tooltip. * accessibility name * appearance of disabled state. It's currently using 0.5 alpha. (may be fine as is) It doesn't have inkdrop like other buttons on the frame. (minimize,maximize,close etc).
,
Jan 10 2018
thanks Oshima. I asked Laura and Sebastien to provide accessibility name, as well as assets. I'm ok to drop the tooltip if works for everyone. Let's keep with 0.5 alpha appearance for disabled state.
,
Jan 10 2018
Laura confirmed, accessibility is 'back button'
,
Jan 12 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/304ec7f27e0a23583a04cb9d0888e328df253ac5 commit 304ec7f27e0a23583a04cb9d0888e328df253ac5 Author: Mitsuru Oshima <oshima@chromium.org> Date: Fri Jan 12 21:41:47 2018 Enable V1app backbutton by default Added about flag in case for troubleshooting. BUG=749713 TEST=manual Change-Id: I2f74535282bb8dccca38a4d8ad6f10af88140c62 Reviewed-on: https://chromium-review.googlesource.com/860522 Commit-Queue: Mitsuru Oshima <oshima@chromium.org> Reviewed-by: Michael Wasserman <msw@chromium.org> Cr-Commit-Position: refs/heads/master@{#529074} [modify] https://crrev.com/304ec7f27e0a23583a04cb9d0888e328df253ac5/ash/public/cpp/ash_switches.cc [modify] https://crrev.com/304ec7f27e0a23583a04cb9d0888e328df253ac5/ash/public/cpp/ash_switches.h [modify] https://crrev.com/304ec7f27e0a23583a04cb9d0888e328df253ac5/chrome/browser/about_flags.cc [modify] https://crrev.com/304ec7f27e0a23583a04cb9d0888e328df253ac5/chrome/browser/flag_descriptions.cc [modify] https://crrev.com/304ec7f27e0a23583a04cb9d0888e328df253ac5/chrome/browser/flag_descriptions.h [modify] https://crrev.com/304ec7f27e0a23583a04cb9d0888e328df253ac5/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc [modify] https://crrev.com/304ec7f27e0a23583a04cb9d0888e328df253ac5/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.h [modify] https://crrev.com/304ec7f27e0a23583a04cb9d0888e328df253ac5/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash_browsertest.cc [modify] https://crrev.com/304ec7f27e0a23583a04cb9d0888e328df253ac5/tools/metrics/histograms/enums.xml
,
Jan 19 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ca91b45e9052a21eceff60a32e8a31beabf5434e commit ca91b45e9052a21eceff60a32e8a31beabf5434e Author: Giovanni Ortuño Urquidi <ortuno@chromium.org> Date: Fri Jan 19 01:41:32 2018 Revert "Enable V1app backbutton by default" This reverts commit 304ec7f27e0a23583a04cb9d0888e328df253ac5. Reason for revert: The original patch enabled the back button on all Hosted App windows. This included Desktop PWA Hosted Apps that had a manifest with display:standalone which explicitly requests for no browser controls. Reverting this change for now until we can disable the back button for Desktop PWA Hosted Apps. Original change's description: > Enable V1app backbutton by default > > Added about flag in case for troubleshooting. > > BUG=749713 > TEST=manual > > Change-Id: I2f74535282bb8dccca38a4d8ad6f10af88140c62 > Reviewed-on: https://chromium-review.googlesource.com/860522 > Commit-Queue: Mitsuru Oshima <oshima@chromium.org> > Reviewed-by: Michael Wasserman <msw@chromium.org> > Cr-Commit-Position: refs/heads/master@{#529074} TBR=msw@chromium.org,oshima@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 749713 Change-Id: I9d405002dc9061d42f4acb7575b4e7c08e8ce925 Reviewed-on: https://chromium-review.googlesource.com/874491 Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org> Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org> Cr-Commit-Position: refs/heads/master@{#530382} [modify] https://crrev.com/ca91b45e9052a21eceff60a32e8a31beabf5434e/ash/public/cpp/ash_switches.cc [modify] https://crrev.com/ca91b45e9052a21eceff60a32e8a31beabf5434e/ash/public/cpp/ash_switches.h [modify] https://crrev.com/ca91b45e9052a21eceff60a32e8a31beabf5434e/chrome/browser/about_flags.cc [modify] https://crrev.com/ca91b45e9052a21eceff60a32e8a31beabf5434e/chrome/browser/flag_descriptions.cc [modify] https://crrev.com/ca91b45e9052a21eceff60a32e8a31beabf5434e/chrome/browser/flag_descriptions.h [modify] https://crrev.com/ca91b45e9052a21eceff60a32e8a31beabf5434e/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc [modify] https://crrev.com/ca91b45e9052a21eceff60a32e8a31beabf5434e/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.h [modify] https://crrev.com/ca91b45e9052a21eceff60a32e8a31beabf5434e/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash_browsertest.cc [modify] https://crrev.com/ca91b45e9052a21eceff60a32e8a31beabf5434e/tools/metrics/histograms/enums.xml
,
Jan 19 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6811e9384e7959657e325f16677eab15acc6d32b commit 6811e9384e7959657e325f16677eab15acc6d32b Author: Mitsuru Oshima <oshima@chromium.org> Date: Fri Jan 19 23:34:55 2018 Update assets/accessible name on back button BUG=749713 TEST=manual Change-Id: I05908a8761336bf6bb8daf24e40201536ab16c40 Reviewed-on: https://chromium-review.googlesource.com/876940 Reviewed-by: Ahmed Fakhry <afakhry@chromium.org> Commit-Queue: Mitsuru Oshima <oshima@chromium.org> Cr-Commit-Position: refs/heads/master@{#530658} [modify] https://crrev.com/6811e9384e7959657e325f16677eab15acc6d32b/ash/ash_strings.grd [modify] https://crrev.com/6811e9384e7959657e325f16677eab15acc6d32b/ash/frame/caption_buttons/frame_back_button.cc [modify] https://crrev.com/6811e9384e7959657e325f16677eab15acc6d32b/ash/resources/vector_icons/window_control_back.1x.icon [modify] https://crrev.com/6811e9384e7959657e325f16677eab15acc6d32b/ash/resources/vector_icons/window_control_back.icon
,
Jan 23 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7cdb4669a382606ae5a4226e8bbc993992934507 commit 7cdb4669a382606ae5a4226e8bbc993992934507 Author: Giovanni Ortuño Urquidi <ortuno@chromium.org> Date: Tue Jan 23 20:58:18 2018 Revert "Enable V1app backbutton by default" This reverts commit 304ec7f27e0a23583a04cb9d0888e328df253ac5. Reason for revert: The original patch enabled the back button on all Hosted App windows. This included Desktop PWA Hosted Apps that had a manifest with display:standalone which explicitly requests for no browser controls. Reverting this change for now until we can disable the back button for Desktop PWA Hosted Apps. Original change's description: > Enable V1app backbutton by default > > Added about flag in case for troubleshooting. > > BUG=749713 > TEST=manual > > Change-Id: I2f74535282bb8dccca38a4d8ad6f10af88140c62 > Reviewed-on: https://chromium-review.googlesource.com/860522 > Commit-Queue: Mitsuru Oshima <oshima@chromium.org> > Reviewed-by: Michael Wasserman <msw@chromium.org> > Cr-Commit-Position: refs/heads/master@{#529074} TBR=msw@chromium.org,oshima@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 749713, 804510 Change-Id: I9d405002dc9061d42f4acb7575b4e7c08e8ce925 Reviewed-on: https://chromium-review.googlesource.com/874491 Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org> Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#530382}(cherry picked from commit ca91b45e9052a21eceff60a32e8a31beabf5434e) Reviewed-on: https://chromium-review.googlesource.com/881981 Cr-Commit-Position: refs/branch-heads/3325@{#36} Cr-Branched-From: bc084a8b5afa3744a74927344e304c02ae54189f-refs/heads/master@{#530369} [modify] https://crrev.com/7cdb4669a382606ae5a4226e8bbc993992934507/ash/public/cpp/ash_switches.cc [modify] https://crrev.com/7cdb4669a382606ae5a4226e8bbc993992934507/ash/public/cpp/ash_switches.h [modify] https://crrev.com/7cdb4669a382606ae5a4226e8bbc993992934507/chrome/browser/about_flags.cc [modify] https://crrev.com/7cdb4669a382606ae5a4226e8bbc993992934507/chrome/browser/flag_descriptions.cc [modify] https://crrev.com/7cdb4669a382606ae5a4226e8bbc993992934507/chrome/browser/flag_descriptions.h [modify] https://crrev.com/7cdb4669a382606ae5a4226e8bbc993992934507/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc [modify] https://crrev.com/7cdb4669a382606ae5a4226e8bbc993992934507/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.h [modify] https://crrev.com/7cdb4669a382606ae5a4226e8bbc993992934507/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash_browsertest.cc [modify] https://crrev.com/7cdb4669a382606ae5a4226e8bbc993992934507/tools/metrics/histograms/enums.xml |
|||||||||||||||
►
Sign in to add a comment |
|||||||||||||||
Comment 1 by kuscher@chromium.org
, Jul 27 2017Components: UI>Shell UI>Shell>WindowManager
Labels: M-61
Owner: ovanieva@chromium.org
Status: Assigned (was: Untriaged)