MD Settings: Restore focus after closing dialogs, for content settings.

MD Settings: Restore focus after closing dialogs, for content settings.

BUG=668313
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2791053002
Cr-Commit-Position: refs/heads/master@{#461597}
Committed: https://chromium.googlesource.com/chromium/src/+/04741e074fcddd1a3b830a85d054034e0b2a34cb

[dpapad, 2017-03-31 23:51:21]

Description was changed from

==========
MD Settings: Restore focus after closing dialogs, for content settings.

BUG=668313
==========

to

==========
MD Settings: Restore focus after closing dialogs, for content settings.

BUG=668313
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[dpapad, 2017-03-31 23:56:25]

The CQ bit was checked by dpapad@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-31 23:56:56]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dpapad, 2017-04-01 00:35:24]

dpapad@chromium.org changed reviewers:
+ dschuyler@chromium.org

[commit-bot: I haz the power, 2017-04-01 01:10:12]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-01 01:10:13]

Dry run: This issue passed the CQ dry run.

[dschuyler, 2017-04-03 21:44:57]

lgtm

https://codereview.chromium.org/2791053002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/site_settings/site_list.js (right):

https://codereview.chromium.org/2791053002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/site_settings/site_list.js:487:
Polymer.dom(/** @type {!Event} */ (e)).localTarget);
There's a lot of text for a simple "A = B" line. Can fewer casts be done?

If we can define e like
* @param {!Event|!{model: !{item: !SiteException}}} e
then maybe we could skip one of the casts. If we could also define Polymer.dom
to return an HTMLElement then this line could be much simpler:

this.activeDialogAnchor_ = Polymer.dom(e).localTarget;

I understand this may be outside of the scope of this CL. No change is required
for LG.

[dpapad, 2017-04-03 23:42:09]

https://codereview.chromium.org/2791053002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/site_settings/site_list.js (right):

https://codereview.chromium.org/2791053002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/site_settings/site_list.js:487:
Polymer.dom(/** @type {!Event} */ (e)).localTarget);
On 2017/04/03 at 21:44:56, dschuyler wrote:
> There's a lot of text for a simple "A = B" line. Can fewer casts be done?
> 
> If we can define e like
> * @param {!Event|!{model: !{item: !SiteException}}} e
> then maybe we could skip one of the casts. If we could also define Polymer.dom
to return an HTMLElement then this line could be much simpler:
> 
> this.activeDialogAnchor_ = Polymer.dom(e).localTarget;
> 
> I understand this may be outside of the scope of this CL. No change is
required for LG.

Tried changing the @param to "{!Event|!{model: !{item: !SiteException}}}", but
it still does not please the compiler. Polymer.dom() does not accept the 2nd
part of the conditional type, and throws the following error

##
/usr/local/google/home/dpapad/workspace/chromium1/src/chrome/browser/resources/settings/site_settings/site_list.js:486:
ERROR - actual parameter 1 of Polymer.dom does not match formal parameter
## found   : (Event|{model: {item: {
##   category: settings.ContentSettingsTypes<string>,
##   controlledBy: string,
##   displayName: string,
##   embeddingDisplayName: string,
##   embeddingOrigin: string,
##   enforcement: string,
##   incognito: boolean,
##   origin: string,
##   setting: string
## }}})
## required: (Event|Node|null)
##     this.activeDialogAnchor_ = /** @type {!HTMLElement} */
(Polymer.dom(e).localTarget);
##                                                                         ^

Also, regarding the 2nd suggestion of making Polymer.dom to return an
HTMLElement, that is also not possible, since it returns different types based
on the input. As revealed by the error code above, you can pass an Event and it
will return an Event, or a Node and it will return a Node.

I agree that the amount of type casts here is unfortunate, and that the compiler
is more of an obstacle than a facilitator in this case (and similar cases across
the codebase).

[dpapad, 2017-04-04 00:04:00]

The CQ bit was checked by dpapad@chromium.org

[commit-bot: I haz the power, 2017-04-04 00:04:24]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-04 01:20:35]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1491264240955660,
"parent_rev": "cef092a92ecbef2465ccf990bfc16db49fbe24fb", "commit_rev":
"04741e074fcddd1a3b830a85d054034e0b2a34cb"}

[commit-bot: I haz the power, 2017-04-04 01:21:26]

Description was changed from

==========
MD Settings: Restore focus after closing dialogs, for content settings.

BUG=668313
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
MD Settings: Restore focus after closing dialogs, for content settings.

BUG=668313
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2791053002
Cr-Commit-Position: refs/heads/master@{#461597}
Committed:
https://chromium.googlesource.com/chromium/src/+/04741e074fcddd1a3b830a85d054...
==========

[commit-bot: I haz the power, 2017-04-04 01:21:27]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/04741e074fcddd1a3b830a85d054...