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

Issue 345743002: Translate: Hide the bubble if the user denies translating 2 times within 24 hours (Closed)

Created:
6 years, 6 months ago by hajimehoshi
Modified:
6 years, 6 months ago
CC:
chromium-reviews, tfarina, droger
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Project:
chromium
Visibility:
Public.

Description

Translate: Hide the bubble if the user denies translating 2 times within 24 hours This CL hides the bubble if the user denies the translation like cliking 'Nope' or clicking somewhere outside of the bubble two times within 24 hours. The state is saved at the (Translate) Preferences and synced. If the state goes to the 'hide' mode, Chrome won't show the Translate bubble automatically any more. In an incognito window, the bubble won't be shown automatically any more. BUG=379035 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=279588

Patch Set 1 #

Total comments: 2

Patch Set 2 : sky's review #

Total comments: 7

Patch Set 3 : Renamed prefs names #

Unified diffs Side-by-side diffs Delta from patch set Stats (+49 lines, -4 lines) Patch
M chrome/browser/translate/chrome_translate_client.cc View 1 2 1 chunk +7 lines, -0 lines 0 comments Download
M components/translate/core/browser/translate_prefs.h View 1 2 2 chunks +8 lines, -0 lines 0 comments Download
M components/translate/core/browser/translate_prefs.cc View 1 2 4 chunks +29 lines, -0 lines 0 comments Download
M components/translate/core/browser/translate_ui_delegate.cc View 1 2 1 chunk +5 lines, -4 lines 0 comments Download

Messages

Total messages: 16 (0 generated)
hajimehoshi
PTAL sky: browser_view.cc toyoshim: the others
6 years, 6 months ago (2014-06-19 11:38:09 UTC) #1
sky
https://codereview.chromium.org/345743002/diff/1/chrome/browser/ui/views/frame/browser_view.cc File chrome/browser/ui/views/frame/browser_view.cc (right): https://codereview.chromium.org/345743002/diff/1/chrome/browser/ui/views/frame/browser_view.cc#newcode1170 chrome/browser/ui/views/frame/browser_view.cc:1170: if (IsOffTheRecord()) This code is not platform specific. Can ...
6 years, 6 months ago (2014-06-19 15:46:46 UTC) #2
hajimehoshi
+droger (CC) Thank you! https://codereview.chromium.org/345743002/diff/1/chrome/browser/ui/views/frame/browser_view.cc File chrome/browser/ui/views/frame/browser_view.cc (right): https://codereview.chromium.org/345743002/diff/1/chrome/browser/ui/views/frame/browser_view.cc#newcode1170 chrome/browser/ui/views/frame/browser_view.cc:1170: if (IsOffTheRecord()) On 2014/06/19 15:46:46, ...
6 years, 6 months ago (2014-06-20 11:38:45 UTC) #3
droger
Some nits: https://codereview.chromium.org/345743002/diff/60001/components/translate/core/browser/translate_prefs.h File components/translate/core/browser/translate_prefs.h (right): https://codereview.chromium.org/345743002/diff/60001/components/translate/core/browser/translate_prefs.h#newcode92 components/translate/core/browser/translate_prefs.h:92: void UpdateBubbleLastClosedTime(); Can we have comments for ...
6 years, 6 months ago (2014-06-20 12:09:29 UTC) #4
hajimehoshi
https://codereview.chromium.org/345743002/diff/60001/components/translate/core/browser/translate_prefs.h File components/translate/core/browser/translate_prefs.h (right): https://codereview.chromium.org/345743002/diff/60001/components/translate/core/browser/translate_prefs.h#newcode93 components/translate/core/browser/translate_prefs.h:93: bool IsBubbleHidden() const; On 2014/06/20 12:09:29, droger wrote: > ...
6 years, 6 months ago (2014-06-20 14:15:46 UTC) #5
Takashi Toyoshima
https://codereview.chromium.org/345743002/diff/60001/components/translate/core/browser/translate_prefs.h File components/translate/core/browser/translate_prefs.h (right): https://codereview.chromium.org/345743002/diff/60001/components/translate/core/browser/translate_prefs.h#newcode93 components/translate/core/browser/translate_prefs.h:93: bool IsBubbleHidden() const; As droger said, having bubble UI ...
6 years, 6 months ago (2014-06-23 11:13:31 UTC) #6
hajimehoshi
Thank you! https://codereview.chromium.org/345743002/diff/60001/components/translate/core/browser/translate_prefs.h File components/translate/core/browser/translate_prefs.h (right): https://codereview.chromium.org/345743002/diff/60001/components/translate/core/browser/translate_prefs.h#newcode93 components/translate/core/browser/translate_prefs.h:93: bool IsBubbleHidden() const; On 2014/06/23 11:13:30, Takashi ...
6 years, 6 months ago (2014-06-24 06:17:54 UTC) #7
droger
lgtm
6 years, 6 months ago (2014-06-24 08:16:41 UTC) #8
hajimehoshi
The CQ bit was checked by hajimehoshi@chromium.org
6 years, 6 months ago (2014-06-24 08:31:44 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/hajimehoshi@chromium.org/345743002/80001
6 years, 6 months ago (2014-06-24 08:32:03 UTC) #10
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: win_chromium_rel on tryserver.chromium ...
6 years, 6 months ago (2014-06-24 10:18:19 UTC) #11
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 6 months ago (2014-06-24 11:38:14 UTC) #12
commit-bot: I haz the power
Try jobs failed on following builders: win_gpu_triggered_tests on tryserver.chromium.gpu (http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu_triggered_tests/builds/21247)
6 years, 6 months ago (2014-06-24 11:38:15 UTC) #13
hajimehoshi
The CQ bit was checked by hajimehoshi@chromium.org
6 years, 6 months ago (2014-06-25 03:34:52 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/hajimehoshi@chromium.org/345743002/80001
6 years, 6 months ago (2014-06-25 03:36:06 UTC) #15
commit-bot: I haz the power
6 years, 6 months ago (2014-06-25 04:46:30 UTC) #16
Message was sent while issue was closed.
Change committed as 279588

Powered by Google App Engine
This is Rietveld 408576698