Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(97)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 520743002: The password bubble should fade out on click on the web page. (Closed)

Created:
6 years, 3 months ago by vasilii
Modified:
6 years, 3 months ago
Reviewers:
vabr (Chromium)
CC:
chromium-reviews, tfarina, gcasto+watchlist_chromium.org, mkwst+watchlist_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Project:
chromium
Visibility:
Public.

Description

The password bubble should fade out on click on the web page. The timeout is now obsolete. The bubble stays infinitely if there is no user input. BUG=394287 Committed: https://crrev.com/a5f25ade2930458735c5041e7f391bed2d401e49 Cr-Commit-Position: refs/heads/master@{#292887}

Patch Set 1 #

Total comments: 2

Patch Set 2 : fixed the nit #

Patch Set 3 : examine the test #

Patch Set 4 : Check if the bubble is destroyed #

Patch Set 5 : Add a stack trace #

Patch Set 6 : Fix the test #

Total comments: 2

Patch Set 7 : fix the nit #

Unified diffs Side-by-side diffs Delta from patch set Stats (+114 lines, -38 lines) Patch
M chrome/browser/ui/views/passwords/manage_passwords_bubble_view.h View 5 chunks +15 lines, -12 lines 0 comments Download
M chrome/browser/ui/views/passwords/manage_passwords_bubble_view.cc View 1 2 3 4 5 10 chunks +48 lines, -21 lines 0 comments Download
M chrome/browser/ui/views/passwords/manage_passwords_bubble_view_browsertest.cc View 1 2 3 4 5 6 5 chunks +51 lines, -5 lines 0 comments Download

Messages

Total messages: 19 (3 generated)
vasilii
vasilii@chromium.org changed reviewers: + vabr@chromium.org
6 years, 3 months ago (2014-08-29 16:24:28 UTC) #1
vasilii
Hi Vaclav, please review.
6 years, 3 months ago (2014-08-29 16:24:28 UTC) #2
vabr (Chromium)
LGTM with a formatting nit. Thanks! Vaclav https://codereview.chromium.org/520743002/diff/1/chrome/browser/ui/views/passwords/manage_passwords_bubble_view.cc File chrome/browser/ui/views/passwords/manage_passwords_bubble_view.cc (right): https://codereview.chromium.org/520743002/diff/1/chrome/browser/ui/views/passwords/manage_passwords_bubble_view.cc#newcode563 chrome/browser/ui/views/passwords/manage_passwords_bubble_view.cc:563: virtual void ...
6 years, 3 months ago (2014-08-29 16:34:52 UTC) #3
vasilii
https://codereview.chromium.org/520743002/diff/1/chrome/browser/ui/views/passwords/manage_passwords_bubble_view.cc File chrome/browser/ui/views/passwords/manage_passwords_bubble_view.cc (right): https://codereview.chromium.org/520743002/diff/1/chrome/browser/ui/views/passwords/manage_passwords_bubble_view.cc#newcode563 chrome/browser/ui/views/passwords/manage_passwords_bubble_view.cc:563: virtual void OnImplicitAnimationsCompleted() OVERRIDE{ On 2014/08/29 16:34:52, vabr (Chromium) ...
6 years, 3 months ago (2014-08-29 16:37:59 UTC) #4
vasilii
The CQ bit was checked by vasilii@chromium.org
6 years, 3 months ago (2014-08-29 16:38:51 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/vasilii@chromium.org/520743002/20001
6 years, 3 months ago (2014-08-29 16:39:39 UTC) #6
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: win_chromium_rel_swarming on tryserver.chromium.win ...
6 years, 3 months ago (2014-08-29 17:40:47 UTC) #7
commit-bot: I haz the power
Try jobs failed on following builders: win_chromium_rel_swarming on tryserver.chromium.win (http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_swarming/builds/7282)
6 years, 3 months ago (2014-08-29 19:13:32 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/vasilii@chromium.org/520743002/40001
6 years, 3 months ago (2014-09-01 10:42:20 UTC) #11
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: win_chromium_x64_rel_swarming on tryserver.chromium.win ...
6 years, 3 months ago (2014-09-01 11:37:26 UTC) #12
vasilii
please review again
6 years, 3 months ago (2014-09-01 16:15:21 UTC) #13
vabr (Chromium)
LGTM, Thanks for the systematic solution. Vaclav https://codereview.chromium.org/520743002/diff/100001/chrome/browser/ui/views/passwords/manage_passwords_bubble_view_browsertest.cc File chrome/browser/ui/views/passwords/manage_passwords_bubble_view_browsertest.cc (right): https://codereview.chromium.org/520743002/diff/100001/chrome/browser/ui/views/passwords/manage_passwords_bubble_view_browsertest.cc#newcode26 chrome/browser/ui/views/passwords/manage_passwords_bubble_view_browsertest.cc:26: // Listens ...
6 years, 3 months ago (2014-09-01 16:20:48 UTC) #14
vasilii
https://codereview.chromium.org/520743002/diff/100001/chrome/browser/ui/views/passwords/manage_passwords_bubble_view_browsertest.cc File chrome/browser/ui/views/passwords/manage_passwords_bubble_view_browsertest.cc (right): https://codereview.chromium.org/520743002/diff/100001/chrome/browser/ui/views/passwords/manage_passwords_bubble_view_browsertest.cc#newcode26 chrome/browser/ui/views/passwords/manage_passwords_bubble_view_browsertest.cc:26: // Listens to WebContents and invokes a callback when ...
6 years, 3 months ago (2014-09-01 16:25:37 UTC) #15
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/vasilii@chromium.org/520743002/120001
6 years, 3 months ago (2014-09-01 16:27:14 UTC) #17
commit-bot: I haz the power
Committed patchset #7 (id:120001) as 3dc8959b66ffb3d33838769a188a86a8072d84ad
6 years, 3 months ago (2014-09-01 17:24:26 UTC) #18
commit-bot: I haz the power
6 years, 3 months ago (2014-09-10 03:17:20 UTC) #19
Message was sent while issue was closed. 
Patchset 7 (id:??) landed as
https://crrev.com/a5f25ade2930458735c5041e7f391bed2d401e49
Cr-Commit-Position: refs/heads/master@{#292887}

Powered by Google App Engine
This is Rietveld 408576698