New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 750577 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Regression :'Old' image is still visible after discarding in chrome://md-settings/changePicture

Project Member Reported by mmanchala@chromium.org, Jul 31 2017

Issue description

Chrome Version: 62.0.369.0/9794.0.0 dev-channel Paine,Kip and Parrot
OS: Chrome

What steps will reproduce the problem?
(1)Sign into User -> Go to 'chrome://md-settings/changePicture' page -> click on  'Take photo' option
(2)Now Capture Photo and observe Image preview is seen misplaced(Please refer Video and screenshot)
(3)Now click on 'Discard photo' and observe unknown Image is seen instead of 'Take photo' screen (Please refer screenshot)

Expected: 
a)After capturing Photo Image preview should not be misplaced(after step 2)
b)After clicking on 'Discard photo' 'Take photo' screen should be seen(after step 3)

Actual: Instead
a)Image preview is seen misplaced(after step 2)
b)Unknown Image is seen and also observe Unknown Image at 'Existing photo from camera or file' (after step 3)

This is Regression Issue as same is working fine in 62.0.3166.0/9785.0.0 dev-channel Kip

@stevenjb : Please confirm the Issue
 
Actual.webm
1.3 MB View Download
Actual_ImagePreviewMisplaced.png
248 KB View Download
Actual_UnknownImageAfterclickingonDiscardPhoto.jpg
131 KB View Download
Expected.webm
1.4 MB View Download
Expected_ImagePreview.png
303 KB View Download
Cc: reve...@chromium.org
Probably a result of https://chromium-review.googlesource.com/585811

I'll try to take a look at this today.

Cc: steve...@chromium.org
Owner: reve...@chromium.org
Status: Fixed (was: Assigned)
This appears to have been fixed.

Status: Assigned (was: Fixed)
Step 2 in Comment # 0 got fixed(Please refer 'Step2isFixed' Attachment) whereas Step 3 is not fixed(Please refer 'Step3isNotFixed' Attachment).Please confirm whether to raise a new Issue or not

Thanks..!!
Step2IsFixed.png
188 KB View Download
Step3isNotFixed.png
128 KB View Download
Actual.webm
489 KB View Download
Owner: steve...@chromium.org
Assigning this to myself to investigate the problem in step 3 which I am able to reproduce.

Summary: Regression :'Old' image is still visible after discarding in chrome://md-settings/changePicture (was: Regression : Image preview is seen misplaced in chrome://md-settings/changePicture)
Project Member

Comment 6 by bugdroid1@chromium.org, Aug 2 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2f12bcecf7c7dd2d24025b453b15feef5a21415d

commit 2f12bcecf7c7dd2d24025b453b15feef5a21415d
Author: Steven Bennetts <stevenjb@chromium.org>
Date: Wed Aug 02 20:00:36 2017

default_user_iamge: Elim invalid DCHECK

In https://chromium-review.googlesource.com/576567 we unifed some of
the default_user_image logic. In doing so it is now possible to call
GetDefaultImageUrl with a negative index. Rather than require all
callers to test their parameters or handle an empty image, this CL
returns the default avatar image for invalid input parameters which
should be handled correctly in the UI.

Bug:  751335 , 750577 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I3f52af21748d37b55fbed95f24ba2ec07e91e311
Reviewed-on: https://chromium-review.googlesource.com/598451
Reviewed-by: David Reveman <reveman@chromium.org>
Commit-Queue: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491477}
[modify] https://crrev.com/2f12bcecf7c7dd2d24025b453b15feef5a21415d/chrome/browser/chromeos/login/users/default_user_image/default_user_images.cc
[modify] https://crrev.com/2f12bcecf7c7dd2d24025b453b15feef5a21415d/chrome/browser/chromeos/login/users/default_user_image/default_user_images.h
[modify] https://crrev.com/2f12bcecf7c7dd2d24025b453b15feef5a21415d/ui/webui/resources/cr_elements/chromeos/cr_picture/cr_picture_list.js
[modify] https://crrev.com/2f12bcecf7c7dd2d24025b453b15feef5a21415d/ui/webui/resources/cr_elements/chromeos/cr_picture/cr_picture_types.js

Status: Fixed (was: Assigned)
Cc: dhadd...@chromium.org mkarkada@chromium.org
Verified on M62 (Chrome OS 9817.0.0, 62.0.3176.0 dev build).
Labels: M-61 Merge-Request-61
Merge request for #6 as required for new default profile avatars (issue 721647).
Project Member

Comment 10 by sheriffbot@chromium.org, Aug 11 2017

Labels: -Merge-Request-61 Merge-Review-61 Hotlist-Merge-Review
This bug requires manual review: M61 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), ketakid@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Review-61 Merge-Approved-61
Approving merge to M61 Chrome OS.
Project Member

Comment 12 by bugdroid1@chromium.org, Aug 12 2017

Labels: -merge-approved-61 merge-merged-3163
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9d6ff31f0142d7701631957aacdb62fb9f363f80

commit 9d6ff31f0142d7701631957aacdb62fb9f363f80
Author: David Reveman <reveman@chromium.org>
Date: Sat Aug 12 13:38:53 2017

default_user_iamge: Elim invalid DCHECK

In https://chromium-review.googlesource.com/576567 we unifed some of
the default_user_image logic. In doing so it is now possible to call
GetDefaultImageUrl with a negative index. Rather than require all
callers to test their parameters or handle an empty image, this CL
returns the default avatar image for invalid input parameters which
should be handled correctly in the UI.

TBR=stevenjb@chromium.org

(cherry picked from commit 2f12bcecf7c7dd2d24025b453b15feef5a21415d)

Bug:  751335 , 750577 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I3f52af21748d37b55fbed95f24ba2ec07e91e311
Reviewed-on: https://chromium-review.googlesource.com/598451
Reviewed-by: David Reveman <reveman@chromium.org>
Commit-Queue: Steven Bennetts <stevenjb@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#491477}
Reviewed-on: https://chromium-review.googlesource.com/612053
Cr-Commit-Position: refs/branch-heads/3163@{#515}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/9d6ff31f0142d7701631957aacdb62fb9f363f80/chrome/browser/chromeos/login/users/default_user_image/default_user_images.cc
[modify] https://crrev.com/9d6ff31f0142d7701631957aacdb62fb9f363f80/chrome/browser/chromeos/login/users/default_user_image/default_user_images.h
[modify] https://crrev.com/9d6ff31f0142d7701631957aacdb62fb9f363f80/ui/webui/resources/cr_elements/chromeos/cr_picture/cr_picture_list.js
[modify] https://crrev.com/9d6ff31f0142d7701631957aacdb62fb9f363f80/ui/webui/resources/cr_elements/chromeos/cr_picture/cr_picture_types.js

Status: Verified (was: Fixed)
9901.66.0, 62.0.3202.82 stable-channel

Sign in to add a comment