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

Issue 608269 link

Starred by 3 users

Issue metadata

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



Sign in to add a comment

Regression: Unwanted grey highlight is seen while taking selected part screenshot

Project Member Reported by sc00335...@techmahindra.com, May 2 2016

Issue description

Version: 52.0.2721.0/8269.0.0 (Official Build) dev-channel peach_pit,peppy,quawks
OS: Chrome os

What steps will reproduce the problem?
(1) Sign in to user/Browse as guest >> Take a screenshot of any selected part by using ctrl+shift+screenshot button
(2) Observe selected part while taking scrrenshot

Expected: Selected part should be transparent while taking screenshot.
Actual: Instead grey highlight is seen while taking selected part screenshot.

This is a regression issue as it is working fine in 49.0.2623.112/7834.70.0 (Official Build) stable-channel daisy.

@afakhry: Please confirm whether it is an issue or intended behavior.

Thanks!
 
Actual.JPG
71.6 KB View Download
Expected.JPG
73.4 KB View Download

Comment 1 by ajha@chromium.org, May 2 2016

Reproducible on 52.0.2721.0/8269.0.0 (Official Build) dev-channel peach_pit
Cc: osh...@chromium.org
This definitely looks like a regression.
Cc: kuscher@chromium.org
I kept this to keep it consistent with window screenshot. kuscher@, I forgot to ask this when I showed you. Please let me know if you prefer to keep the original behavior.

This will not affect the screenshot itself.

Cc: afakhry@chromium.org
Owner: osh...@chromium.org
This looks better to me than the original behavior. Assigning to Oshima to decide what to do.

Comment 5 by osh...@chromium.org, May 13 2016

Owner: kuscher@chromium.org
ksucher@, let me know your decision, and I'll change the behavior.

Comment 6 by osh...@chromium.org, May 21 2016

Cc: warx@chromium.org
Owner: osh...@chromium.org
Status: Started (was: Assigned)
I discussed with kuscher@ and we decided to adjust the UI a bit.

* flip the selected region so that entire screen will be fill with gray, and the selected area is cleared.
* eliminate white rectangle and align the gray region with the region to be taken.
Project Member

Comment 7 by bugdroid1@chromium.org, Jun 2 2016

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

commit ef6f5af980b06e664d0cd9dc7cb578d6862b6c36
Author: oshima <oshima@chromium.org>
Date: Thu Jun 02 20:28:12 2016

Update selection region style

* use simple gray region
* use gray for unselected regions instead.
* use darker gray for unselected region
* Instead of using platform cursor, draw pseudo cursor when selecting region

BUG= 608269 

Review-Url: https://codereview.chromium.org/1997193002
Cr-Commit-Position: refs/heads/master@{#397498}

[modify] https://crrev.com/ef6f5af980b06e664d0cd9dc7cb578d6862b6c36/ash/utility/screenshot_controller.cc
[modify] https://crrev.com/ef6f5af980b06e664d0cd9dc7cb578d6862b6c36/ash/utility/screenshot_controller_unittest.cc

Labels: Merge-Request-52

Comment 9 by tin...@google.com, Jun 3 2016

Labels: -Merge-Request-52 Merge-Approved-52 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M52 (branch: 2743)
Project Member

Comment 10 by bugdroid1@chromium.org, Jun 4 2016

Labels: -merge-approved-52 merge-merged-2743
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a051d657f68b2d60bedf2a91a0edad9b2d2d62c1

commit a051d657f68b2d60bedf2a91a0edad9b2d2d62c1
Author: Mitsuru Oshima <oshima@chromium.org>
Date: Fri Jun 03 23:58:30 2016

Update selection region style

* use simple gray region
* use gray for unselected regions instead.
* use darker gray for unselected region
* Instead of using platform cursor, draw pseudo cursor when selecting region

BUG= 608269 

Review-Url: https://codereview.chromium.org/1997193002
Cr-Commit-Position: refs/heads/master@{#397498}
(cherry picked from commit ef6f5af980b06e664d0cd9dc7cb578d6862b6c36)

Review URL: https://codereview.chromium.org/2030253004 .

Cr-Commit-Position: refs/branch-heads/2743@{#222}
Cr-Branched-From: 2b3ae3b8090361f8af5a611712fc1a5ab2de53cb-refs/heads/master@{#394939}

[modify] https://crrev.com/a051d657f68b2d60bedf2a91a0edad9b2d2d62c1/ash/utility/screenshot_controller.cc
[modify] https://crrev.com/a051d657f68b2d60bedf2a91a0edad9b2d2d62c1/ash/utility/screenshot_controller_unittest.cc

Status: Fixed (was: Started)
Labels: TE-Verified-52.0.2743.32 TE-Verified-M52
Tested the issue on Chrome OS using 52.0.2743.32/8350.21.0 (Official Build) dev-channel peppy.Observed that selected part is transparent while taking screenshot.

Marking it as TE-Verified.
Tested the issue on Chrome OS using 52.0.2743.33/8350.23.0 (Official Build) beta-channel peppy.Observed that selected part is transparent while taking screenshot.

Yep, that's new, expected behavior.
Project Member

Comment 15 by bugdroid1@chromium.org, Jun 13 2016

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

commit 1a5840e8c54cb8995daadbba7300128fd0fc43b0
Author: Mitsuru Oshima <oshima@chromium.org>
Date: Mon Jun 13 23:21:29 2016

Update selection region style

* use simple gray region
* use gray for unselected regions instead.
* use darker gray for unselected region
* Instead of using platform cursor, draw pseudo cursor when selecting region

BUG= 608269 

Review-Url: https://codereview.chromium.org/1997193002
Cr-Commit-Position: refs/heads/master@{#397498}
(cherry picked from commit ef6f5af980b06e664d0cd9dc7cb578d6862b6c36)

Review URL: https://codereview.chromium.org/2030253004 .

Cr-Commit-Position: refs/branch-heads/2743@{#341}
Cr-Branched-From: 2b3ae3b8090361f8af5a611712fc1a5ab2de53cb-refs/heads/master@{#394939}

[modify] https://crrev.com/1a5840e8c54cb8995daadbba7300128fd0fc43b0/content/browser/renderer_host/render_widget_host_view_aura.cc

I screw up commit message. CL was for  crbug.com/618426 .
Project Member

Comment 17 by bugdroid1@chromium.org, Jun 15 2016

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

commit 1a5840e8c54cb8995daadbba7300128fd0fc43b0
Author: Mitsuru Oshima <oshima@chromium.org>
Date: Mon Jun 13 23:21:29 2016

Update selection region style

* use simple gray region
* use gray for unselected regions instead.
* use darker gray for unselected region
* Instead of using platform cursor, draw pseudo cursor when selecting region

BUG= 608269 

Review-Url: https://codereview.chromium.org/1997193002
Cr-Commit-Position: refs/heads/master@{#397498}
(cherry picked from commit ef6f5af980b06e664d0cd9dc7cb578d6862b6c36)

Review URL: https://codereview.chromium.org/2030253004 .

Cr-Commit-Position: refs/branch-heads/2743@{#341}
Cr-Branched-From: 2b3ae3b8090361f8af5a611712fc1a5ab2de53cb-refs/heads/master@{#394939}

[modify] https://crrev.com/1a5840e8c54cb8995daadbba7300128fd0fc43b0/content/browser/renderer_host/render_widget_host_view_aura.cc

Status: Verified (was: Fixed)
Verified on ChromeOS 8350.36.0, 52.0.2743.46 beta and TOT

Sign in to add a comment