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

Issue 653602 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Oct 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug



Sign in to add a comment

Security Verbose Decoration Does Not Animate Back In

Project Member Reported by spqc...@chromium.org, Oct 6 2016

Issue description

OS: Mac

Steps to reproduce:
1) Make sure the material-security-verbose flag is set to "Show all, animated"
2) Navigate to google.com
3) Open a new tab
4) Navigate to amazon.com

What did you expected?
- The secure verbose to animate it

What happened instead?
- It didn't do that
 
I suspect that the verbose is not getting the flag set properly that it has been animated out, and now it should animate back in.

Potentially because of the tab switch. This is likely caused by my refactor from a few weeks ago
Cc: emilyschechter@chromium.org shrike@chromium.org
emilyschechter@ - this bug is probably a side effect of enabling "Show all, animated" for material security verbose. It seems like we wouldn't want to ship with bugs introduced by enabling "Show all, animated", but perhaps this is minor. Wondering what you think.
Interesting, it looks like it animates for some sites and not others. (i.e. I opened google.com in another tab and it did animate, opened ianfette.org in another tab and it did animate, opened amazon.com in another tab and it did not).

I agree we probably wouldn't want to ship with this option. I'm wondering if the new option Sarah is adding (animate non-secure, show secure) has this bug?
Status: Started (was: Assigned)
Sorry for the confusion and slow response, I've been swamped this week.
Anyway, I managed to knock down more tasks that I had expected so I fixed this bug.

I added the fix in this CL: https://codereview.chromium.org/2378623007/
This CL will contain the new flag, renaming it, changing the default to "Show all, non animated" and fixing this animation. The CL will affect both Mac and Views.

So far I got a LGTM from krb and rsesek. I was going to have pkasting to look at it but he's currently OOO so I'll look for someone else. Thanks!
Labels: ReleaseBlock-Stable
Labels: Merge-Request-55

Comment 9 by dimu@chromium.org, Oct 15 2016

Labels: -Merge-Request-55 Merge-Approved-55 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M55 (branch: 2883)
Please merge your change to M55 branch 2883 before 4:00 PM PT, Monday (10/17/16) in order to make to last M55 dev release before Beta promotion. Thank you.
Project Member

Comment 11 by bugdroid1@chromium.org, Oct 17 2016

Labels: -merge-approved-55 merge-merged-2883
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a9f993aeb65b6c5513afe9e8661a88b063b66707

commit a9f993aeb65b6c5513afe9e8661a88b063b66707
Author: spqchan <spqchan@chromium.org>
Date: Mon Oct 17 19:28:56 2016

[Mac] Fix Animation Issue With the Security State Decoration

Reset the decoration's animation if it's hidden

BUG= 653602 

Review-Url: https://codereview.chromium.org/2408253002
Cr-Commit-Position: refs/heads/master@{#425378}
(cherry picked from commit 8ae400d406c3a7e3fb08b47256d4d54fb60fc555)

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

Cr-Commit-Position: refs/branch-heads/2883@{#155}
Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768}

[modify] https://crrev.com/a9f993aeb65b6c5513afe9e8661a88b063b66707/chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.mm
[modify] https://crrev.com/a9f993aeb65b6c5513afe9e8661a88b063b66707/chrome/browser/ui/cocoa/location_bar/security_state_bubble_decoration.h
[modify] https://crrev.com/a9f993aeb65b6c5513afe9e8661a88b063b66707/chrome/browser/ui/cocoa/location_bar/security_state_bubble_decoration.mm

Labels: Needs-Feedback
Verified the issue on Mac 10.11.6 using chrome dev version #55.0.2883.18 as per the comment #0

Steps followed to reproduce the issue are as follows:
---------
1) Set material-security-verbose flag to "Show all, animated"
2) Navigated to google.com
3) Opened a new tab
4) Navigated to amazon.com

Attaching screencast for reference

spqchan@ - Could you please verify the screen cast and let us know if it is the expected behavior.
653602.mp4
3.6 MB View Download
Cc: krajshree@chromium.org
Status: Fixed (was: Started)
Looks good, thanks!
Labels: TE-Verified-M55 TE-Verified-55.0.2883.18
The fix is working as expected as per the comment #14

Hence, adding the verified labels.

Thanks...!!
Project Member

Comment 16 by bugdroid1@chromium.org, Oct 27 2016

Labels: merge-merged-2840
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a9f993aeb65b6c5513afe9e8661a88b063b66707

commit a9f993aeb65b6c5513afe9e8661a88b063b66707
Author: spqchan <spqchan@chromium.org>
Date: Mon Oct 17 19:28:56 2016

[Mac] Fix Animation Issue With the Security State Decoration

Reset the decoration's animation if it's hidden

BUG= 653602 

Review-Url: https://codereview.chromium.org/2408253002
Cr-Commit-Position: refs/heads/master@{#425378}
(cherry picked from commit 8ae400d406c3a7e3fb08b47256d4d54fb60fc555)

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

Cr-Commit-Position: refs/branch-heads/2883@{#155}
Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768}

[modify] https://crrev.com/a9f993aeb65b6c5513afe9e8661a88b063b66707/chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.mm
[modify] https://crrev.com/a9f993aeb65b6c5513afe9e8661a88b063b66707/chrome/browser/ui/cocoa/location_bar/security_state_bubble_decoration.h
[modify] https://crrev.com/a9f993aeb65b6c5513afe9e8661a88b063b66707/chrome/browser/ui/cocoa/location_bar/security_state_bubble_decoration.mm

Comment 17 by dimu@google.com, Nov 4 2016

Labels: -merge-merged-2840
[Automated comment] removing mislabelled merge-merged-2840

Sign in to add a comment