[cros] Profile image no longer selected when user clicks Take Photo/Choose File buttons.

[cros] Profile image no longer selected when user clicks Take Photo/Choose File buttons.


BUG=chromium-os:23237
TEST=Manual: see bug description; choosing any user picture on Change Picture pref page should work.

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

[Ivan Korotkov, 2011-12-08 14:41:01]

PTAL

[whywhat, 2011-12-08 17:27:50]

lgtm

http://codereview.chromium.org/8879015/diff/1/chrome/browser/resources/option...
File chrome/browser/resources/options/chromeos/change_picture_options.js
(right):

http://codereview.chromium.org/8879015/diff/1/chrome/browser/resources/option...
chrome/browser/resources/options/chromeos/change_picture_options.js:124: //
Ignore empty, programmatical and button selection.
I'd rather tell reader here what empty url and programmatical flags actually
mean like:
// Ignore selection change to unset image, caused by program itself or to one of
the action buttons.

http://codereview.chromium.org/8879015/diff/1/chrome/browser/resources/option...
chrome/browser/resources/options/chromeos/change_picture_options.js:125: if
(!imageGrid.inProgramSelection &&
I'd reorder and reformat like this:
if (url &&
    !imageGrid.inProgramSelection &&
    ButtonImageUrls.indexOf(url) == -1) {

to match the order in comment and make it easier to parse.

[Ivan Korotkov, 2011-12-09 09:30:23]

http://codereview.chromium.org/8879015/diff/1/chrome/browser/resources/option...
File chrome/browser/resources/options/chromeos/change_picture_options.js
(right):

http://codereview.chromium.org/8879015/diff/1/chrome/browser/resources/option...
chrome/browser/resources/options/chromeos/change_picture_options.js:124: //
Ignore empty, programmatical and button selection.
On 2011/12/08 17:27:50, whywhat wrote:
> I'd rather tell reader here what empty url and programmatical flags actually
> mean like:
> // Ignore selection change to unset image, caused by program itself or to one
of
> the action buttons.

Done.

http://codereview.chromium.org/8879015/diff/1/chrome/browser/resources/option...
chrome/browser/resources/options/chromeos/change_picture_options.js:125: if
(!imageGrid.inProgramSelection &&
On 2011/12/08 17:27:50, whywhat wrote:
> I'd reorder and reformat like this:
> if (url &&
>     !imageGrid.inProgramSelection &&
>     ButtonImageUrls.indexOf(url) == -1) {
> 
> to match the order in comment and make it easier to parse.

Done.

[Ivan Korotkov, 2011-12-09 09:34:28]

jhawkins: please take a look

[James Hawkins, 2011-12-10 04:23:40]

lgtm