[PrintPreview] Custom input focus toggles

[PrintPreview] Custom input focus toggles

In print preview mode, focus toggles for radio buttons for pagerange
settings when custom input radio button is clicked again and page 
range is empty. 

This is not desired and happens because focus is set for
another radio button on blur event for custom input radio
button when it is empty.

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

[Nikhil, 2014-08-05 14:39:40]

PTAL.

If you've any review comments, then please let me know. I'll incorporate them.

Thanks!

https://codereview.chromium.org/435393005/diff/1/chrome/browser/resources/pri...
File chrome/browser/resources/print_preview/settings/page_settings.js (right):

https://codereview.chromium.org/435393005/diff/1/chrome/browser/resources/pri...
chrome/browser/resources/print_preview/settings/page_settings.js:172:
onCustomInputBlur_: function() {
Do we need to keep this empty function?

[Aleksey Shlyapnikov, 2014-08-07 22:49:15]

https://codereview.chromium.org/435393005/diff/1/chrome/browser/resources/pri...
File chrome/browser/resources/print_preview/settings/page_settings.js (left):

https://codereview.chromium.org/435393005/diff/1/chrome/browser/resources/pri...
chrome/browser/resources/print_preview/settings/page_settings.js:174: if
(this.customInput_.value == '') {
You're breaking the existing functionality, changing selection back to All when
no range is provided. You should check that control other than custom radio
button is focused, something like this:

if (this.customInput_.value == '' && e.relatedTarget != this.customRadio_)

https://codereview.chromium.org/435393005/diff/1/chrome/browser/resources/pri...
File chrome/browser/resources/print_preview/settings/page_settings.js (right):

https://codereview.chromium.org/435393005/diff/1/chrome/browser/resources/pri...
chrome/browser/resources/print_preview/settings/page_settings.js:172:
onCustomInputBlur_: function() {
On 2014/08/05 14:39:40, Nikhil wrote:
> Do we need to keep this empty function?

It should not be empty, see my other comment.

[Nikhil, 2014-08-08 11:34:36]

PTAL. Thanks!

https://codereview.chromium.org/435393005/diff/1/chrome/browser/resources/pri...
File chrome/browser/resources/print_preview/settings/page_settings.js (left):

https://codereview.chromium.org/435393005/diff/1/chrome/browser/resources/pri...
chrome/browser/resources/print_preview/settings/page_settings.js:174: if
(this.customInput_.value == '') {
On 2014/08/07 22:49:14, Aleksey Shlyapnikov wrote:
> You're breaking the existing functionality, changing selection back to All
when
> no range is provided. You should check that control other than custom radio
> button is focused, something like this:
> 
> if (this.customInput_.value == '' && e.relatedTarget != this.customRadio_)

Thanks for reviewing and catching this! I didn't know changing selection back to
All when no range is provided is required. I tried the solution that you
mentioned and it now works as intended.

When no range is provided, clicking custom radio button again doesn't cause
selection to toggle. And if we lose focus (for eg. right click somewhere) when
no range is provided, selection goes back to All.

PTAL. Thanks!

https://codereview.chromium.org/435393005/diff/1/chrome/browser/resources/pri...
File chrome/browser/resources/print_preview/settings/page_settings.js (right):

https://codereview.chromium.org/435393005/diff/1/chrome/browser/resources/pri...
chrome/browser/resources/print_preview/settings/page_settings.js:172:
onCustomInputBlur_: function() {
On 2014/08/07 22:49:14, Aleksey Shlyapnikov wrote:
> On 2014/08/05 14:39:40, Nikhil wrote:
> > Do we need to keep this empty function?
> 
> It should not be empty, see my other comment.

Acknowledged.

[Aleksey Shlyapnikov, 2014-08-08 16:43:16]

lgtm

[Nikhil, 2014-08-08 18:14:50]

vitalybuka@ - OWNERS review please.

[Vitaly Buka (NO REVIEWS), 2014-08-08 18:44:46]

lgtm

[commit-bot: I haz the power, 2014-08-08 18:47:31]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/n.bansal@samsung.com/435393005/20001

[commit-bot: I haz the power, 2014-08-08 23:23:37]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  win_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel/...)

[commit-bot: I haz the power, 2014-08-09 01:39:05]

Change committed as 288485