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

Issue 715400 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Long OOO (go/where-is-mgiuca)
Closed: Aug 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug



Sign in to add a comment

AppBannerPromptResult should be a dictionary, not interface

Project Member Reported by mgiuca@chromium.org, Apr 26 2017

Issue description

Chrome Version: 59
OS: Android

AppBannerPromptResult:

https://cs.chromium.org/chromium/src/third_party/WebKit/Source/modules/app_banner/AppBannerPromptResult.idl

is an interface. Given that it's just a container for two fields, it should be a dictionary, not an interface. (Practically, this means the user would not be able to inspect AppBannerPromptResult as a JavaScript object.)
 

Comment 1 by mgiuca@chromium.org, May 25 2017

Status: Started (was: Assigned)
I sent an Intent to Deprecate and Remove for the interface:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/l-SFJez37x8

And here is the code review:
https://chromium-review.googlesource.com/c/514782/
Project Member

Comment 2 by bugdroid1@chromium.org, Aug 16 2017

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

commit e9dbb63c29071eacc83d2c121a5835a237f1c45d
Author: Matt Giuca <mgiuca@chromium.org>
Date: Wed Aug 16 03:35:00 2017

Converted AppBannerPromptResult from interface to dictionary.

Brings the implementation closer to the spec:
https://www.w3.org/TR/appmanifest/#dom-promptresponseobject

This is a web-facing change (it removes AppBannerPromptResult from the
global namespace) but should not have a material impact since there is
no reason to look at this.

Bug:  715400 
Change-Id: I6bba2e3f00521dcbc58c462a75e921ae703c9b5c
Reviewed-on: https://chromium-review.googlesource.com/514782
Commit-Queue: Matt Giuca <mgiuca@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#494687}
[modify] https://crrev.com/e9dbb63c29071eacc83d2c121a5835a237f1c45d/android_webview/tools/system_webview_shell/test/data/webexposed/global-interface-listing-expected.txt
[modify] https://crrev.com/e9dbb63c29071eacc83d2c121a5835a237f1c45d/third_party/WebKit/LayoutTests/platform/mac/virtual/stable/webexposed/global-interface-listing-expected.txt
[modify] https://crrev.com/e9dbb63c29071eacc83d2c121a5835a237f1c45d/third_party/WebKit/LayoutTests/platform/win/virtual/stable/webexposed/global-interface-listing-expected.txt
[modify] https://crrev.com/e9dbb63c29071eacc83d2c121a5835a237f1c45d/third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt
[delete] https://crrev.com/270598cc07586f4240970ac5101b21f87b26ef84/third_party/WebKit/Source/modules/app_banner/AppBannerPromptResult.cpp
[delete] https://crrev.com/270598cc07586f4240970ac5101b21f87b26ef84/third_party/WebKit/Source/modules/app_banner/AppBannerPromptResult.h
[modify] https://crrev.com/e9dbb63c29071eacc83d2c121a5835a237f1c45d/third_party/WebKit/Source/modules/app_banner/AppBannerPromptResult.idl
[modify] https://crrev.com/e9dbb63c29071eacc83d2c121a5835a237f1c45d/third_party/WebKit/Source/modules/app_banner/BUILD.gn
[modify] https://crrev.com/e9dbb63c29071eacc83d2c121a5835a237f1c45d/third_party/WebKit/Source/modules/app_banner/BeforeInstallPromptEvent.cpp
[modify] https://crrev.com/e9dbb63c29071eacc83d2c121a5835a237f1c45d/third_party/WebKit/Source/modules/app_banner/BeforeInstallPromptEvent.h
[modify] https://crrev.com/e9dbb63c29071eacc83d2c121a5835a237f1c45d/third_party/WebKit/Source/modules/modules_idl_files.gni

Comment 3 by mgiuca@chromium.org, Aug 16 2017

Status: Fixed (was: Started)

Sign in to add a comment