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

Issue 823602 link

Starred by 2 users
Status: Fixed
Owner:
dominickn@chromium.org
Closed: Mar 2018
Cc:
█ mgiuca@chromium.org
dominickn@chromium.org
█ bgalbs@google.com
pcovell@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

BeforeInstallPromptEvent.prompt() should not show a prompt if app already installed

Project Member Reported by mgiuca@chromium.org, Mar 20 2018 
Chrome Version: 66
OS: Chrome (and other desktop)

What steps will reproduce the problem?
(1) Visit https://airhorner.com/
(2) Wait until the Install button shows.
(3) Chrome menu -> Add to Shelf (do *not* use the install button).
(4) Install the app. The browser tab pops out to an app window.
(5) Click the Install button.

What is the expected result?
The Install button should do nothing.

(Note: The app should remove the Install button when it receives the appinstalled event, but that is not a Chrome bug.)

What happens instead?
The Install button shows a second install prompt. Clicking "Add" does nothing.

Comment 1 by dominickn@chromium.org, Mar 20 2018 

Fixing this should just involve making AppBannerManagerDesktop::DidFinishCreatingBookmarkApp() call AppBannerManager::ResetBindings(), which resets all the mojo bindings.

Comment 2 by dominickn@chromium.org, Mar 20 2018 

Hmm wait, since this is using Add to Shelf, the call is better made in AppBannerManager::OnInstall(), which is called by the Add to Shelf code path.
Project Member

Comment 3 by bugdroid1@chromium.org, Mar 20 2018 

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

commit 10f218c6c9b29d8a163c87a7b6a725a39bf2d07f
Author: Dominick Ng <dominickn@chromium.org>
Date: Tue Mar 20 09:50:24 2018

Reset event bindings in AppBannerManager::OnInstall.

This addresses a bug where sites using the beforeinstallpromptevent to
implement an install button could have users trigger a do-nothing
install prompt if the user first used the menu item "Add to Shelf" to
install, and then pressed the beforeinstallprompt-connected button.

Resetting the event binding when the OnInstall() method is triggered
means that when an install is started, all outstanding
beforeinstallpromptevents are voided, fixing the bug.

BUG= 823602 

Change-Id: Ib2c747c74f7083a7663580ed37da7a319d9a9c60
Reviewed-on: https://chromium-review.googlesource.com/970050
Reviewed-by: Matt Giuca <mgiuca@chromium.org>
Commit-Queue: Dominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544317}
[modify] https://crrev.com/10f218c6c9b29d8a163c87a7b6a725a39bf2d07f/chrome/browser/banners/app_banner_manager.cc

Comment 4 by mgiuca@chromium.org, Mar 20 2018

Cc: -dominickn@chromium.org mgiuca@chromium.org
Owner: dominickn@chromium.org
Status: Fixed (was: Assigned)
►

Sign in to add a comment