Add network proxy input to md-settings internet details

Add network proxy input to md-settings internet details

BUG=481563
Committed: https://crrev.com/d9ca8bd6f034b0b9b6e8f31b28ab43b3442b18f0
Cr-Commit-Position: refs/heads/master@{#338383}

[stevenjb, 2015-07-08 01:59:25]

https://codereview.chromium.org/1219753007/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/internet_page/internet_detail_page.css
(right):

https://codereview.chromium.org/1219753007/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/internet_page/internet_detail_page.css:38:
max-width: 500px;
needed?

https://codereview.chromium.org/1219753007/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/internet_page/internet_detail_page.html
(right):

https://codereview.chromium.org/1219753007/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/internet_page/internet_detail_page.html:1:
<link rel="import" href="chrome://resources/polymer/v1_0/polymer/polymer.html">
+layout

https://codereview.chromium.org/1219753007/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/internet_page/internet_detail_page.html:82:
<network-proxy editable
elim 'editable' for now

https://codereview.chromium.org/1219753007/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/internet_page/internet_detail_page.js
(right):

https://codereview.chromium.org/1219753007/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/internet_page/internet_detail_page.js:360: *
@param {!{detail: { field: string, value: string|Object}}} event
value: CrOnc.ProxySettings

https://codereview.chromium.org/1219753007/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/internet_page/internet_detail_page.js:373: };
one line

https://codereview.chromium.org/1219753007/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/internet_page/network_proxy.html (right):

https://codereview.chromium.org/1219753007/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/internet_page/network_proxy.html:26:
on-change="onProxyInputChange_">
test

https://codereview.chromium.org/1219753007/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/internet_page/network_proxy.html:32: <span>Web
Proxy Auto Discovery URL:</span><span>[[WPAD]]</span>
Set WPAD from ipconfig

https://codereview.chromium.org/1219753007/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/internet_page/network_proxy.js (right):

https://codereview.chromium.org/1219753007/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/internet_page/network_proxy.js:96:
this.$$('#selectType').value = proxy.Type;
comment

https://codereview.chromium.org/1219753007/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/internet_page/network_proxy.js:106: 
comment

https://codereview.chromium.org/1219753007/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/internet_page/network_proxy.js:155: 
comment

https://codereview.chromium.org/1219753007/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/internet_page/network_proxy.js:176: 
comment

[stevenjb, 2015-07-08 19:08:54]

Patchset #2 (id:20001) has been deleted

[stevenjb, 2015-07-08 19:11:00]

stevenjb@chromium.org changed reviewers:
+ kalman@chromium.org, michaelpg@chromium.org

[stevenjb, 2015-07-08 19:11:01]

+kalman@ for minor fix to networking_private.idl

[stevenjb, 2015-07-08 19:11:26]

michaelpg@ - Please take a look when you have a chance; no rush.

[not at google - send to devlin, 2015-07-08 19:19:31]

idl change lgtm

[michaelpg, 2015-07-10 00:38:31]

https://codereview.chromium.org/1219753007/diff/40001/chrome/browser/resource...
File chrome/browser/resources/settings/internet_page/network_proxy.html (right):

https://codereview.chromium.org/1219753007/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/network_proxy.html:16: <select
id="selectType" on-change="onTypeChange_">
nit: onTypeSelected, not to be confused with a type property observer.

https://codereview.chromium.org/1219753007/diff/40001/chrome/browser/resource...
File chrome/browser/resources/settings/internet_page/network_proxy.js (right):

https://codereview.chromium.org/1219753007/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/network_proxy.js:81: DIRECT:
CrOnc.ProxySettingsType.DIRECT,
It's sad that we have to do this :-(

https://codereview.chromium.org/1219753007/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/network_proxy.js:115:
this.$$('#selectType').value = proxy.Type;
this.$.selectType

https://codereview.chromium.org/1219753007/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/network_proxy.js:172:
this.copyProxyLocation_(defaultProxy, this.proxy.Manual.SecureHTTPProxy);
Does setting this way break data binding?

https://codereview.chromium.org/1219753007/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/network_proxy.js:189:
onAddProxyExclusion_: function(event) {
nit: annotate this & below

https://codereview.chromium.org/1219753007/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/network_proxy.js:190: var value
= this.$$('#proxyExclusion').value;
this.$

https://codereview.chromium.org/1219753007/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/network_proxy.js:195:
this.$$('#proxyExclusion').value = '';
this.$

https://codereview.chromium.org/1219753007/diff/40001/chrome/browser/resource...
File chrome/browser/resources/settings/internet_page/network_proxy_exclusions.js
(right):

https://codereview.chromium.org/1219753007/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/network_proxy_exclusions.js:39:
this.fire('removed', index);
Can you do the splice here instead?

https://codereview.chromium.org/1219753007/diff/40001/chrome/browser/resource...
File chrome/browser/resources/settings/internet_page/network_proxy_input.js
(right):

https://codereview.chromium.org/1219753007/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/network_proxy_input.js:51:
this.fire('changed', { value: this.value });
'changed' rather than 'change' is a confusing distinction. Events are usually
... imperative? infinitive?

[stevenjb, 2015-07-10 17:31:59]

PTAL

https://codereview.chromium.org/1219753007/diff/40001/chrome/browser/resource...
File chrome/browser/resources/settings/internet_page/network_proxy.html (right):

https://codereview.chromium.org/1219753007/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/network_proxy.html:16: <select
id="selectType" on-change="onTypeChange_">
On 2015/07/10 00:38:30, michaelpg wrote:
> nit: onTypeSelected, not to be confused with a type property observer.

Yeah, but it's an on-change event. I don't disagreee that it's confusing, but I
think it's confusing either way. And DGMS on on-change vs. on-changed. For now I
am trying to match the method name to the event.

https://codereview.chromium.org/1219753007/diff/40001/chrome/browser/resource...
File chrome/browser/resources/settings/internet_page/network_proxy.js (right):

https://codereview.chromium.org/1219753007/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/network_proxy.js:81: DIRECT:
CrOnc.ProxySettingsType.DIRECT,
On 2015/07/10 00:38:30, michaelpg wrote:
> It's sad that we have to do this :-(

Acknowledged.

https://codereview.chromium.org/1219753007/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/network_proxy.js:115:
this.$$('#selectType').value = proxy.Type;
On 2015/07/10 00:38:30, michaelpg wrote:
> this.$.selectType

Done.

https://codereview.chromium.org/1219753007/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/network_proxy.js:172:
this.copyProxyLocation_(defaultProxy, this.proxy.Manual.SecureHTTPProxy);
On 2015/07/10 00:38:30, michaelpg wrote:
> Does setting this way break data binding?

Good catch, you're right, it won't trigger a change notification, which could
result in a subtle bug. Changed to use set() and Object.assign.

https://codereview.chromium.org/1219753007/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/network_proxy.js:189:
onAddProxyExclusion_: function(event) {
On 2015/07/10 00:38:30, michaelpg wrote:
> nit: annotate this & below

Done.

https://codereview.chromium.org/1219753007/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/network_proxy.js:190: var value
= this.$$('#proxyExclusion').value;
On 2015/07/10 00:38:30, michaelpg wrote:
> this.$

Done.

https://codereview.chromium.org/1219753007/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/network_proxy.js:195:
this.$$('#proxyExclusion').value = '';
On 2015/07/10 00:38:30, michaelpg wrote:
> this.$

Done.

https://codereview.chromium.org/1219753007/diff/40001/chrome/browser/resource...
File chrome/browser/resources/settings/internet_page/network_proxy_exclusions.js
(right):

https://codereview.chromium.org/1219753007/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/network_proxy_exclusions.js:39:
this.fire('removed', index);
On 2015/07/10 00:38:30, michaelpg wrote:
> Can you do the splice here instead?

Done.

https://codereview.chromium.org/1219753007/diff/40001/chrome/browser/resource...
File chrome/browser/resources/settings/internet_page/network_proxy_input.js
(right):

https://codereview.chromium.org/1219753007/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/network_proxy_input.js:51:
this.fire('changed', { value: this.value });
On 2015/07/10 00:38:30, michaelpg wrote:
> 'changed' rather than 'change' is a confusing distinction. Events are usually
> ... imperative? infinitive?

I agree. I'm not sure where I got on-changed initially, but unfortunately that
is what I have been using in settings/internet. I have a cleanup CL following
this one where I will replace all 'changed' events with 'change' including this
one.

[michaelpg, 2015-07-10 18:40:38]

lgtm

[stevenjb, 2015-07-10 21:24:52]

The CQ bit was checked by stevenjb@chromium.org

[stevenjb, 2015-07-10 21:24:52]

The patchset sent to the CQ was uploaded after l-g-t-m from kalman@chromium.org,
michaelpg@chromium.org
Link to the patchset: https://codereview.chromium.org/1219753007/#ps80001
(title: "Rebase")

[commit-bot: I haz the power, 2015-07-10 21:26:45]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1219753007/80001

[commit-bot: I haz the power, 2015-07-10 22:34:31]

Committed patchset #4 (id:80001)

[commit-bot: I haz the power, 2015-07-10 22:35:20]

Patchset 4 (id:??) landed as
https://crrev.com/d9ca8bd6f034b0b9b6e8f31b28ab43b3442b18f0
Cr-Commit-Position: refs/heads/master@{#338383}