Move proxy handling into network details.

Move proxy handling into network details.

BUG=chromium:116977
TEST=Open network options for a WiFi or Ethernet connection.  The dialog should contain a proxy tab.  If the network is shared, the contents of the tab are enabled/disabled based on whether "enable proxies for shared networks" is checked in the main settings page.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=126221

[kevers, 2012-03-09 17:51:10]

Hi Chris,

Can you please have a look at this CL, which moves the proxy dialog into the
network details dialog.  Should be 3 extra file deletions are part of this CL
(proxy.html, proxy.css, and proxy_options.js); however, deletions currently
choke the presubmit scripts. :(  Would like to handle the deletions with a
separate CL once the scripts have been fixes.

[csilv, 2012-03-09 21:34:43]

On 2012/03/09 17:51:10, kevers wrote:
> Hi Chris,
> 
> Can you please have a look at this CL, which moves the proxy dialog into the
> network details dialog.  Should be 3 extra file deletions are part of this CL
> (proxy.html, proxy.css, and proxy_options.js); however, deletions currently
> choke the presubmit scripts. :(  Would like to handle the deletions with a
> separate CL once the scripts have been fixes.

Kevin, I assume the files that are going to be deleted are proxy_settings.css,
proxy_settings.html, proxy_settings.js.  Is that correct?

[csilv, 2012-03-09 21:50:40]

On 2012/03/09 21:34:43, csilv wrote:
> On 2012/03/09 17:51:10, kevers wrote:
> > Hi Chris,
> > 
> > Can you please have a look at this CL, which moves the proxy dialog into the
> > network details dialog.  Should be 3 extra file deletions are part of this
CL
> > (proxy.html, proxy.css, and proxy_options.js); however, deletions currently
> > choke the presubmit scripts. :(  Would like to handle the deletions with a
> > separate CL once the scripts have been fixes.
> 
> Kevin, I assume the files that are going to be deleted are proxy_settings.css,
> proxy_settings.html, proxy_settings.js.  Is that correct?

Nevermind, I was looking in the wrong direction.  Can you clarify whether
resources/chromeos/proxy_settings.html/css/js are still used?

[csilv, 2012-03-09 22:30:13]

http://codereview.chromium.org/9668010/diff/2001/chrome/browser/resources/opt...
File chrome/browser/resources/options2/chromeos/internet_detail.css (right):

http://codereview.chromium.org/9668010/diff/2001/chrome/browser/resources/opt...
chrome/browser/resources/options2/chromeos/internet_detail.css:46: 
nit: remove this extra linefeed

http://codereview.chromium.org/9668010/diff/2001/chrome/browser/resources/opt...
File chrome/browser/resources/options2/chromeos/internet_detail.html (right):

http://codereview.chromium.org/9668010/diff/2001/chrome/browser/resources/opt...
chrome/browser/resources/options2/chromeos/internet_detail.html:63: </span>
nit: it looks like line 63 could fit at the end of 62.

http://codereview.chromium.org/9668010/diff/2001/chrome/browser/resources/opt...
chrome/browser/resources/options2/chromeos/internet_detail.html:433: <input
id="proxyHostPort" type="text" data-type="number" size="4"
nit, wrap to 80 cols, here and line 449

http://codereview.chromium.org/9668010/diff/2001/chrome/browser/resources/opt...
chrome/browser/resources/options2/chromeos/internet_detail.html:528: </div>
Consider adding a TODO for a style cleanup.. like lowercase id tags.

http://codereview.chromium.org/9668010/diff/2001/chrome/browser/resources/opt...
File chrome/browser/resources/options2/chromeos/internet_detail.js (right):

http://codereview.chromium.org/9668010/diff/2001/chrome/browser/resources/opt...
chrome/browser/resources/options2/chromeos/internet_detail.js:33:
DetailsInternetPage.prototype.updateControls_;
should be 'updateControls'.

http://codereview.chromium.org/9668010/diff/2001/chrome/browser/resources/opt...
chrome/browser/resources/options2/chromeos/internet_detail.js:206:
this.addEventListener('visibleChange', this.handleVisibleChange_);
handleVisibleChange_ is undefined.

http://codereview.chromium.org/9668010/diff/2001/chrome/browser/resources/opt...
chrome/browser/resources/options2/chromeos/internet_detail.js:335: },
would it be possible to use 'hidden' instead of setting the display property?

[kevers, 2012-03-12 13:57:13]

http://codereview.chromium.org/9668010/diff/2001/chrome/browser/resources/opt...
File chrome/browser/resources/options2/chromeos/internet_detail.css (right):

http://codereview.chromium.org/9668010/diff/2001/chrome/browser/resources/opt...
chrome/browser/resources/options2/chromeos/internet_detail.css:46: 
On 2012/03/09 22:30:14, csilv wrote:
> nit: remove this extra linefeed

Done.

http://codereview.chromium.org/9668010/diff/2001/chrome/browser/resources/opt...
File chrome/browser/resources/options2/chromeos/internet_detail.html (right):

http://codereview.chromium.org/9668010/diff/2001/chrome/browser/resources/opt...
chrome/browser/resources/options2/chromeos/internet_detail.html:63: </span>
On 2012/03/09 22:30:14, csilv wrote:
> nit: it looks like line 63 could fit at the end of 62.

Done.

http://codereview.chromium.org/9668010/diff/2001/chrome/browser/resources/opt...
chrome/browser/resources/options2/chromeos/internet_detail.html:433: <input
id="proxyHostPort" type="text" data-type="number" size="4"
On 2012/03/09 22:30:14, csilv wrote:
> nit, wrap to 80 cols, here and line 449

Done.

http://codereview.chromium.org/9668010/diff/2001/chrome/browser/resources/opt...
chrome/browser/resources/options2/chromeos/internet_detail.html:528: </div>
On 2012/03/09 22:30:14, csilv wrote:
> Consider adding a TODO for a style cleanup.. like lowercase id tags.

Done.

http://codereview.chromium.org/9668010/diff/2001/chrome/browser/resources/opt...
File chrome/browser/resources/options2/chromeos/internet_detail.js (right):

http://codereview.chromium.org/9668010/diff/2001/chrome/browser/resources/opt...
chrome/browser/resources/options2/chromeos/internet_detail.js:33:
DetailsInternetPage.prototype.updateControls_;
On 2012/03/09 22:30:14, csilv wrote:
> should be 'updateControls'.

Done.

http://codereview.chromium.org/9668010/diff/2001/chrome/browser/resources/opt...
chrome/browser/resources/options2/chromeos/internet_detail.js:206:
this.addEventListener('visibleChange', this.handleVisibleChange_);
On 2012/03/09 22:30:14, csilv wrote:
> handleVisibleChange_ is undefined.

Done.

http://codereview.chromium.org/9668010/diff/2001/chrome/browser/resources/opt...
chrome/browser/resources/options2/chromeos/internet_detail.js:335: },
On 2012/03/09 22:30:14, csilv wrote:
> would it be possible to use 'hidden' instead of setting the display property?

Done.

[csilv, 2012-03-12 18:44:35]

lgtm

https://chromiumcodereview.appspot.com/9668010/diff/7001/chrome/browser/resou...
File chrome/browser/resources/options2/chromeos/internet_detail.html (right):

https://chromiumcodereview.appspot.com/9668010/diff/7001/chrome/browser/resou...
chrome/browser/resources/options2/chromeos/internet_detail.html:448: <input
id="secureProxyPort" type="text" data-type="number" size="4"
nit: wrap to 80 cols

[kevers, 2012-03-12 19:55:10]

https://chromiumcodereview.appspot.com/9668010/diff/7001/chrome/browser/resou...
File chrome/browser/resources/options2/chromeos/internet_detail.html (right):

https://chromiumcodereview.appspot.com/9668010/diff/7001/chrome/browser/resou...
chrome/browser/resources/options2/chromeos/internet_detail.html:448: <input
id="secureProxyPort" type="text" data-type="number" size="4"
On 2012/03/12 18:44:35, csilv wrote:
> nit: wrap to 80 cols

Done.

[commit-bot: I haz the power, 2012-03-12 19:55:37]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kevers@chromium.org/9668010/11001

[commit-bot: I haz the power, 2012-03-12 21:12:11]

Change committed as 126221