App install banner: cancel button does not trigger/resolve to a userChoice |
|||||
Issue descriptionChrome Version: 66.0.3330.0 OS: Mac What steps will reproduce the problem? (1) Goto https://killer-marmot.appspot.com/web/ (2) Trigger 'Add to Homescreen' in the DevTools Application Panel (3) Click the "Show the prompt after all." link that the app creates in the event log (4) Click "Add" in the install banne4r (5) Click the "Cancel" button. What is the expected result? The prompt() promise resolves with userChoice === 'dismissed'. What happens instead? The prompt() promise does not resolve.
,
Jan 25 2018
We figured it out earlier this afternoon. The problem is that AppBannerManagerDesktop::DidFinishCreatingBookmarkApp() doesn't call SendBannerDismissed() if the |extension| pointer it gets back is null (signifying that the install failed or the user declined). This fix is straightforward. We should: - make that method call SendBannerDismissed(), which will resolve the promise correctly. - think about passing back the reason for the failure (optional) This affects all desktop platforms, so upping priority.
,
Jan 25 2018
Thanks for the report.
Note that in the repro steps, you may not get a "Show the prompt after all"; there is a 50% chance for it to cancel the automated prompt. So just refresh until you see that button.
The site is a bit confusing with its output logs. Just FYI, the expected output is that it prints:
platform is: 'web'
outcome is: 'dismissed'
The actual output is, it prints nothing.
So yes, this is a bug, and I think it was introduced by the change to automatically re-prompt. Dom's advice looks correct to me. I'll deal with this.
,
Mar 1 2018
,
Mar 12 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/533a7829796d765942b2e66e2ee736140141f05c commit 533a7829796d765942b2e66e2ee736140141f05c Author: Dominick Ng <dominickn@chromium.org> Date: Mon Mar 12 00:43:37 2018 Resolve userChoice promise when cancelling an app banner on desktop. The result of creating a bookmark app is a pointer to an extension. When this pointer is null, either the extension failed to be created, or the user did not accept the installation prompt. This CL assumes that the user declined the prompt, which is the expected outcome of a nullptr. It means that the userChoice promise will be resolved with 'dismissed' when the ExperimentalAppBanners feature is active on desktop. BUG= 805744 Change-Id: Ibafe6478e665169194da930ab4d210e62b07b759 Reviewed-on: https://chromium-review.googlesource.com/942582 Commit-Queue: Dominick Ng <dominickn@chromium.org> Reviewed-by: Matt Giuca <mgiuca@chromium.org> Cr-Commit-Position: refs/heads/master@{#542411} [modify] https://crrev.com/533a7829796d765942b2e66e2ee736140141f05c/chrome/browser/banners/app_banner_manager_desktop.cc
,
Mar 12 2018
Should be fixed and in Canary tomorrow.
,
Mar 14 2018
Verified the fix on Mac 10.13.1, Windows-10 and Ubuntu 14.04 using Chrome version #67.0.3370.0 as per the comment #0. Attaching screen cast for reference. Observed 'dismissed' is triggered at the end. Hence, the fix is working as expected. Adding the verified labels. Note: Able to reproduce the issue on chrome version with out fix. Thanks...!! |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by benwells@chromium.org
, Jan 25 2018Owner: mgiuca@chromium.org