Add autoclick to settings page under the accessibility section.

Add autoclick to settings page under the accessibility section.

In the settings page, we have both a checkbox to enable/disable the feature and a dropdown menu to choose the delay. This feature is hidden behind a flag.

BUG=272401
TEST=manual

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=230075

[Tim Song, 2013-10-15 00:20:23]

+jhawkins for chrome/browser/resources/options/ and
              chrome/browser/ui/webui/options/
+xiyuan@ for chrome/browser/chromeos/
+oshima@ for ash/autoclick/ and
             chromeos/

[xiyuan, 2013-10-15 15:52:33]

https://codereview.chromium.org/27196006/diff/5001/chrome/browser/resources/o...
File chrome/browser/resources/options/browser_options.css (right):

https://codereview.chromium.org/27196006/diff/5001/chrome/browser/resources/o...
chrome/browser/resources/options/browser_options.css:414: float: left;
Is there a way to not use "float"? If you have to use it, you need to handle RTL
as well.

https://codereview.chromium.org/27196006/diff/5001/chrome/browser/resources/o...
chrome/browser/resources/options/browser_options.css:420: float: left;
same here.

https://codereview.chromium.org/27196006/diff/5001/chrome/browser/resources/o...
File chrome/browser/resources/options/browser_options.html (right):

https://codereview.chromium.org/27196006/diff/5001/chrome/browser/resources/o...
chrome/browser/resources/options/browser_options.html:760:
pref="settings.a11y.autoclick">
<span> does not need to bind to a pref. Since you have i18n-content on <label>,
this <span> is probably not needed.

https://codereview.chromium.org/27196006/diff/5001/chrome/browser/resources/o...
chrome/browser/resources/options/browser_options.html:781: </span>
This <span> could be removed too.

[oshima, 2013-10-15 17:04:48]

https://codereview.chromium.org/27196006/diff/5001/ash/autoclick/autoclick_co...
File ash/autoclick/autoclick_controller.h (right):

https://codereview.chromium.org/27196006/diff/5001/ash/autoclick/autoclick_co...
ash/autoclick/autoclick_controller.h:11: const int kDefaultAutoclickDelayMs =
500;
can you move this to static memeber of AutoclickCOntroller ?

https://codereview.chromium.org/27196006/diff/5001/chromeos/chromeos_switches.h
File chromeos/chromeos_switches.h (right):

https://codereview.chromium.org/27196006/diff/5001/chromeos/chromeos_switches...
chromeos/chromeos_switches.h:42: CHROMEOS_EXPORT extern const char
kEnableAutoclick[];
auto click is ash feature, so this should be
ash/ash_switches instead.

[Tim Song, 2013-10-15 22:23:30]

https://codereview.chromium.org/27196006/diff/5001/ash/autoclick/autoclick_co...
File ash/autoclick/autoclick_controller.h (right):

https://codereview.chromium.org/27196006/diff/5001/ash/autoclick/autoclick_co...
ash/autoclick/autoclick_controller.h:11: const int kDefaultAutoclickDelayMs =
500;
On 2013/10/15 17:04:48, oshima wrote:
> can you move this to static memeber of AutoclickCOntroller ?

Done.

https://codereview.chromium.org/27196006/diff/5001/chrome/browser/resources/o...
File chrome/browser/resources/options/browser_options.css (right):

https://codereview.chromium.org/27196006/diff/5001/chrome/browser/resources/o...
chrome/browser/resources/options/browser_options.css:414: float: left;
On 2013/10/15 15:52:33, xiyuan wrote:
> Is there a way to not use "float"? If you have to use it, you need to handle
RTL
> as well.

Done. I never considered the RTL case, thanks!

Other than using a table, I can't really think of another way to align the
checkbox label with the dropdown subsetting.

https://codereview.chromium.org/27196006/diff/5001/chrome/browser/resources/o...
chrome/browser/resources/options/browser_options.css:420: float: left;
On 2013/10/15 15:52:33, xiyuan wrote:
> same here.

Done.

https://codereview.chromium.org/27196006/diff/5001/chrome/browser/resources/o...
File chrome/browser/resources/options/browser_options.html (right):

https://codereview.chromium.org/27196006/diff/5001/chrome/browser/resources/o...
chrome/browser/resources/options/browser_options.html:760:
pref="settings.a11y.autoclick">
On 2013/10/15 15:52:33, xiyuan wrote:
> <span> does not need to bind to a pref. Since you have i18n-content on
<label>,
> this <span> is probably not needed.

Don't we need this controlled-setting-indicator to do a bubble pop-up if the
preference has been changed through sync?

https://codereview.chromium.org/27196006/diff/5001/chrome/browser/resources/o...
chrome/browser/resources/options/browser_options.html:781: </span>
On 2013/10/15 15:52:33, xiyuan wrote:
> This <span> could be removed too.

Don't we need a bubble here here as well?

https://codereview.chromium.org/27196006/diff/5001/chromeos/chromeos_switches.h
File chromeos/chromeos_switches.h (right):

https://codereview.chromium.org/27196006/diff/5001/chromeos/chromeos_switches...
chromeos/chromeos_switches.h:42: CHROMEOS_EXPORT extern const char
kEnableAutoclick[];
On 2013/10/15 17:04:48, oshima wrote:
> auto click is ash feature, so this should be
> ash/ash_switches instead.

Done.

[oshima, 2013-10-15 22:29:24]

ash/ lgtm

https://codereview.chromium.org/27196006/diff/26001/ash/ash_switches.cc
File ash/ash_switches.cc (right):

https://codereview.chromium.org/27196006/diff/26001/ash/ash_switches.cc#newco...
ash/ash_switches.cc:95: const char kAshEnableAutoclick[]               =
"ash-enable-autoclick";
one space between ] and =

[xiyuan, 2013-10-15 23:59:37]

LGTM

Bonus point to get rid of the float.

https://codereview.chromium.org/27196006/diff/5001/chrome/browser/resources/o...
File chrome/browser/resources/options/browser_options.css (right):

https://codereview.chromium.org/27196006/diff/5001/chrome/browser/resources/o...
chrome/browser/resources/options/browser_options.css:414: float: left;
On 2013/10/15 22:23:31, Tim Song wrote:
> On 2013/10/15 15:52:33, xiyuan wrote:
> > Is there a way to not use "float"? If you have to use it, you need to handle
> RTL
> > as well.
> 
> Done. I never considered the RTL case, thanks!
> 
> Other than using a table, I can't really think of another way to align the
> checkbox label with the dropdown subsetting.

I put a CL https://codereview.chromium.org/26238007/ that attempts to do this
without using float. The idea is to put the checkbox label and select subrow in
a the same div to make them align.

https://codereview.chromium.org/27196006/diff/5001/chrome/browser/resources/o...
File chrome/browser/resources/options/browser_options.html (right):

https://codereview.chromium.org/27196006/diff/5001/chrome/browser/resources/o...
chrome/browser/resources/options/browser_options.html:760:
pref="settings.a11y.autoclick">
On 2013/10/15 22:23:31, Tim Song wrote:
> On 2013/10/15 15:52:33, xiyuan wrote:
> > <span> does not need to bind to a pref. Since you have i18n-content on
> <label>,
> > this <span> is probably not needed.
> 
> Don't we need this controlled-setting-indicator to do a bubble pop-up if the
> preference has been changed through sync?

I see. Totally missed that.

[Tim Song, 2013-10-16 19:41:46]

https://codereview.chromium.org/27196006/diff/5001/chrome/browser/resources/o...
File chrome/browser/resources/options/browser_options.css (right):

https://codereview.chromium.org/27196006/diff/5001/chrome/browser/resources/o...
chrome/browser/resources/options/browser_options.css:414: float: left;
On 2013/10/15 23:59:37, xiyuan wrote:
> On 2013/10/15 22:23:31, Tim Song wrote:
> > On 2013/10/15 15:52:33, xiyuan wrote:
> > > Is there a way to not use "float"? If you have to use it, you need to
handle
> > RTL
> > > as well.
> > 
> > Done. I never considered the RTL case, thanks!
> > 
> > Other than using a table, I can't really think of another way to align the
> > checkbox label with the dropdown subsetting.
> 
> I put a CL https://codereview.chromium.org/26238007/ that attempts to do this
> without using float. The idea is to put the checkbox label and select subrow
in
> a the same div to make them align.

Ah, I wasn't too familiar with -webkit-box, -webkit-box-orient, and
-webkit-box-align. I changed the layout to do as you suggested and removed the
floats.

https://codereview.chromium.org/27196006/diff/26001/ash/ash_switches.cc
File ash/ash_switches.cc (right):

https://codereview.chromium.org/27196006/diff/26001/ash/ash_switches.cc#newco...
ash/ash_switches.cc:95: const char kAshEnableAutoclick[]               =
"ash-enable-autoclick";
On 2013/10/15 22:29:25, oshima wrote:
> one space between ] and =

Done.

[xiyuan, 2013-10-16 20:21:39]

LGTM++

Thanks for cleaning up float.

https://codereview.chromium.org/27196006/diff/45001/chrome/browser/resources/...
File chrome/browser/resources/options/browser_options.html (right):

https://codereview.chromium.org/27196006/diff/45001/chrome/browser/resources/...
chrome/browser/resources/options/browser_options.html:754: <span>
Think we can change this to a <div> and remove the css rules to force it block.

[Tim Song, 2013-10-16 21:21:00]

https://codereview.chromium.org/27196006/diff/45001/chrome/browser/resources/...
File chrome/browser/resources/options/browser_options.html (right):

https://codereview.chromium.org/27196006/diff/45001/chrome/browser/resources/...
chrome/browser/resources/options/browser_options.html:754: <span>
On 2013/10/16 20:21:40, xiyuan wrote:
> Think we can change this to a <div> and remove the css rules to force it
block.

Done. .controlled-setting-with-label assumes a span after an input. I just added
a div below the span.

[xiyuan, 2013-10-16 21:34:11]

Thanks for trying it out.

https://codereview.chromium.org/27196006/diff/45001/chrome/browser/resources/...
File chrome/browser/resources/options/browser_options.html (right):

https://codereview.chromium.org/27196006/diff/45001/chrome/browser/resources/...
chrome/browser/resources/options/browser_options.html:754: <span>
On 2013/10/16 21:21:00, Tim Song wrote:
> On 2013/10/16 20:21:40, xiyuan wrote:
> > Think we can change this to a <div> and remove the css rules to force it
> block.
> 
> Done. .controlled-setting-with-label assumes a span after an input. I just
added
> a div below the span.

Okay.

[commit-bot: I haz the power, 2013-10-22 04:00:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tengs@chromium.org/27196006/191001

[commit-bot: I haz the power, 2013-10-22 08:53:43]

Change committed as 230075