New issue
Advanced search Search tips

Issue 854268 link

Starred by 1 user

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 2
Type: Bug

Blocked on:
issue 908705



Sign in to add a comment

Eliminate cr.EventTarget in favor of native EventTarget.

Project Member Reported by dpa...@chromium.org, Jun 19 2018

Issue description

As of Chrome 64, EventTarget can be constructed with "new EventTarget()", see [1]. There is no need anymore for custom EventTarget implementtations like cr.EventTarget at [2].

[1] https://www.chromestatus.com/features/5721972856061952
[2] https://cs.chromium.org/chromium/src/ui/webui/resources/js/cr/event_target.js
 

Comment 1 by dpa...@chromium.org, Jun 19 2018

Description: Show this description
This is blocked on https://github.com/google/closure-compiler/pull/2988.

Basically Closure Compiler currently does not allow calling "new EventTarget()", throws the following error:

JSC_NOT_A_CONSTRUCTOR: cannot instantiate non-constructor 

Status: ExternalDependency (was: Available)
Also filed https://github.com/google/closure-compiler/issues/3143 to track any progress on the JS compiler side.
Owner: dpa...@chromium.org
There seems to be a reasonable workaround, see https://chromium-review.googlesource.com/c/chromium/src/+/1313852. It does require though updating some of the WebUI code, especially within the old Print Preview. Given that old PP is scheduled to be deleted soon, I'll hold off until right after this is done.
Blockedon: 908705
Status: Assigned (was: ExternalDependency)
Status update: In order to use the native EventTarget, need to convert all existing subclasess to ES6 class syntax, otherwise the following error is thrown

"TypeError: Illegal constructor"

Will be converting such cases little by little.

Status: Started (was: Assigned)
Initial CL at https://chromium-review.googlesource.com/c/chromium/src/+/1379307.

Next CL will address the following files in ui/webui/resources/

js/cr/ui/list_selection_controller.js
js/cr/ui/page_manager/page.js
js/cr/ui/context_menu_handler.js
js/cr/ui/table/table_column.js
js/cr/ui/table/table_column_model.js
js/cr/ui/list_selection_model.js
js/cr/ui/list_single_selection_model.js
js/cr/ui/array_data_model.js

Project Member

Comment 8 by bugdroid1@chromium.org, Dec 19

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

commit 6680d29e6247da54685df3c93db519ad861760fe
Author: dpapad <dpapad@chromium.org>
Date: Wed Dec 19 22:31:35 2018

WebUI: Convert a few cr.EventTarget subclasses to ES6 syntax.

This is in preparation of aliasing cr.EventTarget to the native EventTarget
class, and removing the previous custom implementation.

Bug: 854268
Change-Id: I61a15d4a17ee7cd5fe0ce94544b2e2079889ebf4
Reviewed-on: https://chromium-review.googlesource.com/c/1379307
Reviewed-by: Rebekah Potter <rbpotter@chromium.org>
Reviewed-by: Achuith Bhandarkar <achuith@chromium.org>
Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#617986}
[modify] https://crrev.com/6680d29e6247da54685df3c93db519ad861760fe/chrome/browser/resources/bluetooth_internals/adapter_broker.js
[modify] https://crrev.com/6680d29e6247da54685df3c93db519ad861760fe/chrome/browser/resources/gaia_auth_host/authenticator.js
[modify] https://crrev.com/6680d29e6247da54685df3c93db519ad861760fe/chrome/browser/resources/gaia_auth_host/saml_handler.js

Sign in to add a comment