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

Issue 887824 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 27
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome
Pri: 1
Type: Bug



Sign in to add a comment

Don't remove "Create Shortcut" menu option for installable PWA sites

Project Member Reported by alancutter@chromium.org, Sep 21

Issue description

Chrome Version: 70
OS: All

With the launch of DesktopPWAWindowing we replace the "Create shortcut" menu option with "Install <app name>" for installable PWA sites.
Based on user feedback (https://plus.google.com/u/0/+RodneyCarpenter/posts/NyWgE1DnGVG) and internal discussions we've decided to make "Install <app name>" appear in addition to "Create shortcut" so as not to remove existing functionality.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Sep 25

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

commit 81ac726681d099fb940643290274074fb9eda11c
Author: Alan Cutter <alancutter@chromium.org>
Date: Tue Sep 25 00:18:36 2018

Add "Create shortcut..." back to "More tools" when PWA installation is enabled

This CL brings back "Create shortcut..." to the app menu's "More tools" section
even on installable PWA sites where we would originally omit it in favour
of showing "Install <app name>...".
Now we always show "Create shortcut..." and optionally show
"Install <app name>..." if the site is an installable PWA.

This splits the IDC_CREATE_HOSTED_APP command in two; IDC_CREATE_SHORTCUT
and IDC_INSTALL_PWA and plumbs a bool through to BookmarkAppHelper to
distinguish which type of install the user requested.

Bug:  887824 
Change-Id: I432670327588f6639f374289664b3add4a6b7b81
Reviewed-on: https://chromium-review.googlesource.com/1226481
Reviewed-by: Ben Wells <benwells@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Reviewed-by: Finnur Thorarinsson <finnur@chromium.org>
Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593767}
[modify] https://crrev.com/81ac726681d099fb940643290274074fb9eda11c/chrome/app/chrome_command_ids.h
[modify] https://crrev.com/81ac726681d099fb940643290274074fb9eda11c/chrome/browser/banners/app_banner_manager.h
[modify] https://crrev.com/81ac726681d099fb940643290274074fb9eda11c/chrome/browser/extensions/bookmark_app_helper.cc
[modify] https://crrev.com/81ac726681d099fb940643290274074fb9eda11c/chrome/browser/extensions/bookmark_app_helper.h
[modify] https://crrev.com/81ac726681d099fb940643290274074fb9eda11c/chrome/browser/extensions/tab_helper.cc
[modify] https://crrev.com/81ac726681d099fb940643290274074fb9eda11c/chrome/browser/extensions/tab_helper.h
[modify] https://crrev.com/81ac726681d099fb940643290274074fb9eda11c/chrome/browser/ui/browser_command_controller.cc
[modify] https://crrev.com/81ac726681d099fb940643290274074fb9eda11c/chrome/browser/ui/browser_commands.cc
[modify] https://crrev.com/81ac726681d099fb940643290274074fb9eda11c/chrome/browser/ui/browser_commands.h
[modify] https://crrev.com/81ac726681d099fb940643290274074fb9eda11c/chrome/browser/ui/browser_dialogs.h
[modify] https://crrev.com/81ac726681d099fb940643290274074fb9eda11c/chrome/browser/ui/extensions/hosted_app_browsertest.cc
[modify] https://crrev.com/81ac726681d099fb940643290274074fb9eda11c/chrome/browser/ui/toolbar/app_menu_model.cc
[modify] https://crrev.com/81ac726681d099fb940643290274074fb9eda11c/chrome/browser/ui/views/extensions/bookmark_app_confirmation_view.cc
[modify] https://crrev.com/81ac726681d099fb940643290274074fb9eda11c/chrome/browser/ui/views/extensions/pwa_confirmation_view.cc

For reference here are screenshots of the "Create shortcut" / "Install <app name>" menu item on Windows in 69, 70 and 71.
69menu.png
19.7 KB View Download
70menu.png
21.6 KB View Download
71menu.png
18.9 KB View Download
Labels: Merge-Request-70 M-70
Requesting to merge this change to M70 to avoid the menu churn.
Project Member

Comment 4 by sheriffbot@chromium.org, Sep 26

Labels: -Merge-Request-70 Merge-Review-70 Hotlist-Merge-Review
This bug requires manual review: M70 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), geohsu@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Review-70 Merge-Approved-70
Checked with pcovell@ offline - this is approved by UX, safe merge, and needed for M70. Approving merge to M70. branch:3538
Project Member

Comment 6 by bugdroid1@chromium.org, Sep 27

Labels: -merge-approved-70 merge-merged-3538
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3ad358ad5defa72f07f439b4eb5451f5d900f5de

commit 3ad358ad5defa72f07f439b4eb5451f5d900f5de
Author: Alan Cutter <alancutter@chromium.org>
Date: Thu Sep 27 01:08:00 2018

Add "Create shortcut..." back to "More tools" when PWA installation is enabled

This CL brings back "Create shortcut..." to the app menu's "More tools" section
even on installable PWA sites where we would originally omit it in favour
of showing "Install <app name>...".
Now we always show "Create shortcut..." and optionally show
"Install <app name>..." if the site is an installable PWA.

This splits the IDC_CREATE_HOSTED_APP command in two; IDC_CREATE_SHORTCUT
and IDC_INSTALL_PWA and plumbs a bool through to BookmarkAppHelper to
distinguish which type of install the user requested.

TBR=alancutter@chromium.org

(cherry picked from commit 81ac726681d099fb940643290274074fb9eda11c)

Bug:  887824 
Change-Id: I432670327588f6639f374289664b3add4a6b7b81
Reviewed-on: https://chromium-review.googlesource.com/1226481
Reviewed-by: Ben Wells <benwells@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Reviewed-by: Finnur Thorarinsson <finnur@chromium.org>
Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#593767}
Reviewed-on: https://chromium-review.googlesource.com/1247602
Reviewed-by: Alan Cutter <alancutter@chromium.org>
Cr-Commit-Position: refs/branch-heads/3538@{#695}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
[modify] https://crrev.com/3ad358ad5defa72f07f439b4eb5451f5d900f5de/chrome/app/chrome_command_ids.h
[modify] https://crrev.com/3ad358ad5defa72f07f439b4eb5451f5d900f5de/chrome/browser/banners/app_banner_manager.h
[modify] https://crrev.com/3ad358ad5defa72f07f439b4eb5451f5d900f5de/chrome/browser/extensions/bookmark_app_helper.cc
[modify] https://crrev.com/3ad358ad5defa72f07f439b4eb5451f5d900f5de/chrome/browser/extensions/bookmark_app_helper.h
[modify] https://crrev.com/3ad358ad5defa72f07f439b4eb5451f5d900f5de/chrome/browser/extensions/tab_helper.cc
[modify] https://crrev.com/3ad358ad5defa72f07f439b4eb5451f5d900f5de/chrome/browser/extensions/tab_helper.h
[modify] https://crrev.com/3ad358ad5defa72f07f439b4eb5451f5d900f5de/chrome/browser/ui/browser_command_controller.cc
[modify] https://crrev.com/3ad358ad5defa72f07f439b4eb5451f5d900f5de/chrome/browser/ui/browser_commands.cc
[modify] https://crrev.com/3ad358ad5defa72f07f439b4eb5451f5d900f5de/chrome/browser/ui/browser_commands.h
[modify] https://crrev.com/3ad358ad5defa72f07f439b4eb5451f5d900f5de/chrome/browser/ui/browser_dialogs.h
[modify] https://crrev.com/3ad358ad5defa72f07f439b4eb5451f5d900f5de/chrome/browser/ui/extensions/hosted_app_browsertest.cc
[modify] https://crrev.com/3ad358ad5defa72f07f439b4eb5451f5d900f5de/chrome/browser/ui/toolbar/app_menu_model.cc
[modify] https://crrev.com/3ad358ad5defa72f07f439b4eb5451f5d900f5de/chrome/browser/ui/views/extensions/bookmark_app_confirmation_view.cc
[modify] https://crrev.com/3ad358ad5defa72f07f439b4eb5451f5d900f5de/chrome/browser/ui/views/extensions/pwa_confirmation_view.cc

Labels: Merge-Merged-70-3538
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/3ad358ad5defa72f07f439b4eb5451f5d900f5de

Commit: 3ad358ad5defa72f07f439b4eb5451f5d900f5de
Author: alancutter@chromium.org
Commiter: alancutter@chromium.org
Date: 2018-09-27 01:08:00 +0000 UTC

Add "Create shortcut..." back to "More tools" when PWA installation is enabled

This CL brings back "Create shortcut..." to the app menu's "More tools" section
even on installable PWA sites where we would originally omit it in favour
of showing "Install <app name>...".
Now we always show "Create shortcut..." and optionally show
"Install <app name>..." if the site is an installable PWA.

This splits the IDC_CREATE_HOSTED_APP command in two; IDC_CREATE_SHORTCUT
and IDC_INSTALL_PWA and plumbs a bool through to BookmarkAppHelper to
distinguish which type of install the user requested.

TBR=alancutter@chromium.org

(cherry picked from commit 81ac726681d099fb940643290274074fb9eda11c)

Bug:  887824 
Change-Id: I432670327588f6639f374289664b3add4a6b7b81
Reviewed-on: https://chromium-review.googlesource.com/1226481
Reviewed-by: Ben Wells <benwells@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Reviewed-by: Finnur Thorarinsson <finnur@chromium.org>
Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#593767}
Reviewed-on: https://chromium-review.googlesource.com/1247602
Reviewed-by: Alan Cutter <alancutter@chromium.org>
Cr-Commit-Position: refs/branch-heads/3538@{#695}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
Status: Fixed (was: Started)

Sign in to add a comment