Small: Cloud print UI options fix.

Cloud print options fix.

There seemed to be a timing issue on Mac and non-views Linux build that
prevented the button handlers from being properly installed. By always
installing the handlers and basing their behavior on the visibility of the ui
instead, things work on all platforms.

BUG=none
TEST=Start up Mac Chromium with --enable-cloud-print-proxy, go to Under the Hood, and try to start cloud print - with this patch, the sign in UI will start.

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

[Scott Byer, 2011-02-18 18:15:31]

A small fix I found was needed as I start trying to get the Mac cloud print
proxy UI going.

[csilv, 2011-02-18 18:59:54]

http://codereview.chromium.org/6541027/diff/1/chrome/browser/resources/option...
File chrome/browser/resources/options/options.html (right):

http://codereview.chromium.org/6541027/diff/1/chrome/browser/resources/option...
chrome/browser/resources/options/options.html:3: i18n-values="dir:textdirection"
If enable-cloud-print-proxy is no longer defined here, wont that break the style
declarations in options_page.css?  ie:

  html[enable-cloud-print-proxy=false] #cloud-print-proxy-section {
    display: none;
  }

[Scott Byer, 2011-02-18 19:11:32]

http://codereview.chromium.org/6541027/diff/1/chrome/browser/resources/option...
File chrome/browser/resources/options/options.html (right):

http://codereview.chromium.org/6541027/diff/1/chrome/browser/resources/option...
chrome/browser/resources/options/options.html:3: i18n-values="dir:textdirection"

On 2011/02/18 18:59:54, csilv wrote:
> If enable-cloud-print-proxy is no longer defined here, wont that break the
style
> declarations in options_page.css?  ie:
> 
>   html[enable-cloud-print-proxy=false] #cloud-print-proxy-section {
>     display: none;
>   }

I had thought it might, but I tried it, and the section still gets properly
hidden when enable-cloud-print-proxy is false.

[csilv, 2011-02-18 19:52:33]

Ok right.  It still works because when cloud printing is disabled, the handler
calls the 'RemoveCloudPrintProxySection()' method that takes that section out of
the HTML entirely.  Since those CSS declarations in options_page.css are now
obsolete, let's remove them to avoid future confusion.

On 2011/02/18 19:11:32, Scott Byer wrote:
>
http://codereview.chromium.org/6541027/diff/1/chrome/browser/resources/option...
> File chrome/browser/resources/options/options.html (right):
> 
>
http://codereview.chromium.org/6541027/diff/1/chrome/browser/resources/option...
> chrome/browser/resources/options/options.html:3:
i18n-values="dir:textdirection"
> 
> On 2011/02/18 18:59:54, csilv wrote:
> > If enable-cloud-print-proxy is no longer defined here, wont that break the
> style
> > declarations in options_page.css?  ie:
> > 
> >   html[enable-cloud-print-proxy=false] #cloud-print-proxy-section {
> >     display: none;
> >   }
> 
> I had thought it might, but I tried it, and the section still gets properly
> hidden when enable-cloud-print-proxy is false.

[csilv, 2011-02-18 19:54:28]

http://codereview.chromium.org/6541027/diff/1/chrome/browser/resources/option...
File chrome/browser/resources/options/advanced_options.js (right):

http://codereview.chromium.org/6541027/diff/1/chrome/browser/resources/option...
chrome/browser/resources/options/advanced_options.js:133: if
($('cloud-print-proxy-section').style.display != 'none') {
This added check is unnecessary.  The entire section will be removed if cloud
printing is disabled, so any onclick handler could never possibly be called.

http://codereview.chromium.org/6541027/diff/1/chrome/browser/resources/option...
chrome/browser/resources/options/advanced_options.js:146: if
($('cloud-print-proxy-section').style.display != 'none') {
Same with this check.

[Scott Byer, 2011-02-18 20:44:05]

Ah, of course. This make things a bit cleaner...

http://codereview.chromium.org/6541027/diff/1/chrome/browser/resources/option...
File chrome/browser/resources/options/advanced_options.js (right):

http://codereview.chromium.org/6541027/diff/1/chrome/browser/resources/option...
chrome/browser/resources/options/advanced_options.js:133: if
($('cloud-print-proxy-section').style.display != 'none') {
On 2011/02/18 19:54:28, csilv wrote:
> This added check is unnecessary.  The entire section will be removed if cloud
> printing is disabled, so any onclick handler could never possibly be called.

Done.

http://codereview.chromium.org/6541027/diff/1/chrome/browser/resources/option...
chrome/browser/resources/options/advanced_options.js:146: if
($('cloud-print-proxy-section').style.display != 'none') {
On 2011/02/18 19:54:28, csilv wrote:
> Same with this check.

Done.

[csilv, 2011-02-18 22:11:55]

LGTM

http://codereview.chromium.org/6541027/diff/4002/chrome/browser/printing/clou...
File chrome/browser/printing/cloud_print/cloud_print_setup_flow.cc (right):

http://codereview.chromium.org/6541027/diff/4002/chrome/browser/printing/clou...
chrome/browser/printing/cloud_print/cloud_print_setup_flow.cc:136: #if
!defined(OS_WIN)
You may want to add a comment describing why this is necessary for the benefit
of future coders that may wonder.