[Android]Month picker simplification

Simplify the month picker even more by making it locale agnostic for everything except the month names

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

[Miguel Garcia, 2013-05-24 16:48:35]

As agreed, this is part two of the simplifcation which builds on top of
https://codereview.chromium.org/15965004/ still on the submit queue.

[Miguel Garcia, 2013-05-28 14:25:25]

Friendly ping on this patch

[bulach, 2013-05-28 14:43:20]

lgtm but... is this really correct?! I would expect ui-elements to be
locale-aware, not agnostic.. it obviously simplifies things, but I wonder if the
user would expect Jan-Dec in a "month picker" rather than their own locale?

anwways.. just nits below, if this is the right approach, fine by me.

https://codereview.chromium.org/16023004/diff/2001/content/public/android/jav...
File
content/public/android/java/src/org/chromium/content/browser/input/MonthPicker.java
(right):

https://codereview.chromium.org/16023004/diff/2001/content/public/android/jav...
content/public/android/java/src/org/chromium/content/browser/input/MonthPicker.java:66:
Calendar minCal = Calendar.getInstance();
nit: why the locals? is it possible to use mMinDate and mMaxDate directly?

https://codereview.chromium.org/16023004/diff/2001/content/public/android/jav...
content/public/android/java/src/org/chromium/content/browser/input/MonthPicker.java:88:
if (oldVal == (MONTHS_NUMBER -1) && newVal == 0) {
nit: space after -, here and 90 / 91 below.

https://codereview.chromium.org/16023004/diff/2001/content/public/android/jav...
content/public/android/java/src/org/chromium/content/browser/input/MonthPicker.java:170:
mCurrentDate.set(year, month, 1);
nit: how about moving 170 to an "else" block, and removing the redundant
mCurrentDate.clear() from 172 and 176 below?

https://codereview.chromium.org/16023004/diff/2001/content/public/android/jav...
content/public/android/java/src/org/chromium/content/browser/input/MonthPicker.java:186:
mMonthSpinner.setMaxValue(MONTHS_NUMBER -1);
nit: space after -

[bulach, 2013-05-28 14:44:01]

lgtm still, and also, please, make the patch description have a title with
<=80cols, then the longer description.

[Miguel Garcia, 2013-05-28 17:09:53]

Description and title also adjusted. 

Thanks for the review!

https://codereview.chromium.org/16023004/diff/2001/content/public/android/jav...
File
content/public/android/java/src/org/chromium/content/browser/input/MonthPicker.java
(right):

https://codereview.chromium.org/16023004/diff/2001/content/public/android/jav...
content/public/android/java/src/org/chromium/content/browser/input/MonthPicker.java:66:
Calendar minCal = Calendar.getInstance();
On 2013/05/28 14:43:20, bulach wrote:
> nit: why the locals? is it possible to use mMinDate and mMaxDate directly?

Well, to gurantee that they don't have an invalid value at any point in time.
Calendar.getInstance() gets initialize to the current time which might not be in
the allowed range

https://codereview.chromium.org/16023004/diff/2001/content/public/android/jav...
content/public/android/java/src/org/chromium/content/browser/input/MonthPicker.java:88:
if (oldVal == (MONTHS_NUMBER -1) && newVal == 0) {
On 2013/05/28 14:43:20, bulach wrote:
> nit: space after -, here and 90 / 91 below.

Done.

https://codereview.chromium.org/16023004/diff/2001/content/public/android/jav...
content/public/android/java/src/org/chromium/content/browser/input/MonthPicker.java:170:
mCurrentDate.set(year, month, 1);
On 2013/05/28 14:43:20, bulach wrote:
> nit: how about moving 170 to an "else" block, and removing the redundant
> mCurrentDate.clear() from 172 and 176 below?

Makes sense, done

https://codereview.chromium.org/16023004/diff/2001/content/public/android/jav...
content/public/android/java/src/org/chromium/content/browser/input/MonthPicker.java:186:
mMonthSpinner.setMaxValue(MONTHS_NUMBER -1);
On 2013/05/28 14:43:20, bulach wrote:
> nit: space after -

Done.

[commit-bot: I haz the power, 2013-05-28 17:10:16]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/miguelg@chromium.org/16023004/13001

[commit-bot: I haz the power, 2013-05-28 20:14:44]

Change committed as 202635