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

Issue 749713 link

Starred by 4 users

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

Add back button to windowed websites

Project Member Reported by jennschen@chromium.org, Jul 27 2017

Issue description

For 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"
 
Cc: abodenha@chromium.org osh...@chromium.org skuhne@chromium.org
Components: UI>Shell UI>Shell>WindowManager
Labels: M-61
Owner: ovanieva@chromium.org
Status: Assigned (was: Untriaged)
Not sure this is 61 but sure would be nice to be. Should be a small fix
I really like this suggestion. We can target all gSuite apps to start with. 
We can triage with ENG and agree on timelines. 
Cc: zork@chromium.org
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.

Comment 5 by osh...@chromium.org, 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)
The back button would be browser back since it would be acting on a webpage.
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.
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.
Status: WontFix (was: Assigned)
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.
Cc: omrilio@chromium.org
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.

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. 
I can't image how it can work because it doesn't work in normal case. What app did you use?
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. 
I just talked to Omri, and he misunderstood chrome app vs web app. So it still does not work on chrome apps.
Owner: ----
Status: Available (was: WontFix)
Summary: Add back button to windowed websites (was: Add back button to windowed Chrome apps)
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
Description: Show this description
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. 
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

Owner: ovanieva@chromium.org
Status: Assigned (was: Available)
to clarify - this is a request to add back button to streamlined hosted apps. 
Labels: -Pri-2 -M-61 M-64 Pri-3
Owner: ----
Status: Available (was: Assigned)
Owner: ovanieva@chromium.org
Labels: -M-64 M-66
I'm adding back button for ARC++, so it shouldn't be hard to enable probably in 64 or 65 at least.
Labels: -M-66 M-65
thanks Oshima - udpated to M65.
Owner: osh...@chromium.org
Status: Started (was: Available)
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.
Project Member

Comment 28 by bugdroid1@chromium.org, 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

Project Member

Comment 29 by bugdroid1@chromium.org, 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

Owner: ovanieva@chromium.org
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).
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. 
Cc: lpalmaro@chromium.org
Laura confirmed, accessibility is 'back button'
Project Member

Comment 33 by bugdroid1@chromium.org, 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

Project Member

Comment 34 by bugdroid1@chromium.org, 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

Project Member

Comment 36 by bugdroid1@chromium.org, Jan 23 2018

Labels: merge-merged-3325
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