Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(1)

Issue 13818036: Do not need to set suppress_next_commit_ to false in HandleCommit() method, (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 6 months ago by joone
Modified:
4 years, 6 months ago
CC:
chromium-reviews, yusukes+watch_chromium.org, penghuang+watch_chromium.org, joi+watch-content_chromium.org, darin-cc_chromium.org
Base URL:
https://src.chromium.org/chrome/trunk/src/
Visibility:
Public.

Description

We don't need to set suppress_next_commit_ to false in HandleCommit() method, because this method can be called twice when using iBus. In this case, a compositing Korean Hangul character can be committed twice. Contributed by joone.hur@intel.com BUG=50485 TEST=Follow the bug description. Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=195916

Patch Set 1 #

Patch Set 2 : #

Messages

Total messages: 19 (0 generated)
joone
Hello Ben, 50485 bug is reproducible with iBus 1.4.1 on my Ubuntu 12.04, but there ...
4 years, 6 months ago (2013-04-13 08:37:47 UTC) #1
Ben Goodger (Google)
-> erg for further triage.
4 years, 6 months ago (2013-04-16 16:28:42 UTC) #2
Elliot Glaysher
Adding suzhe and penghuang from the bug.
4 years, 6 months ago (2013-04-16 18:15:37 UTC) #3
Elliot Glaysher
suzhe and penghuang: one week ping. If you absolutely don't have time, I can review ...
4 years, 6 months ago (2013-04-22 21:06:40 UTC) #4
James Su
On 2013/04/22 21:06:40, Elliot Glaysher wrote: > suzhe and penghuang: one week ping. If you ...
4 years, 6 months ago (2013-04-23 01:50:24 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/joone.hur@intel.com/13818036/1
4 years, 6 months ago (2013-04-23 02:41:18 UTC) #6
commit-bot: I haz the power
Failed to apply patch for AUTHORS: While running patch -p0 --forward --force --no-backup-if-mismatch; patching file ...
4 years, 6 months ago (2013-04-23 02:41:20 UTC) #7
Elliot Glaysher
Could you please upload a new copy of this patch with a merged AUTHORS file?
4 years, 6 months ago (2013-04-23 16:50:41 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/joone.hur@intel.com/13818036/10002
4 years, 6 months ago (2013-04-23 17:08:08 UTC) #9
commit-bot: I haz the power
Presubmit check for 13818036-10002 failed and returned exit status 1. INFO:root:Found 2 file(s). Running presubmit ...
4 years, 6 months ago (2013-04-23 17:08:10 UTC) #10
joone
On 2013/04/23 16:50:41, Elliot Glaysher wrote: > Could you please upload a new copy of ...
4 years, 6 months ago (2013-04-23 17:08:32 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/joone.hur@intel.com/13818036/10002
4 years, 6 months ago (2013-04-23 17:16:29 UTC) #12
commit-bot: I haz the power
Presubmit check for 13818036-10002 failed and returned exit status 1. INFO:root:Found 2 file(s). Running presubmit ...
4 years, 6 months ago (2013-04-23 17:16:31 UTC) #13
joone
I got a LGTM from James Su, but my patch can not be committed because ...
4 years, 6 months ago (2013-04-23 17:37:21 UTC) #14
Avi (ping after 24h)
lgtm stamp
4 years, 6 months ago (2013-04-23 17:40:01 UTC) #15
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/joone.hur@intel.com/13818036/10002
4 years, 6 months ago (2013-04-23 17:40:13 UTC) #16
jam
On 2013/04/23 17:37:21, joone wrote: > I got a LGTM from James Su, but my ...
4 years, 6 months ago (2013-04-23 20:28:35 UTC) #17
commit-bot: I haz the power
Change committed as 195916
4 years, 6 months ago (2013-04-23 22:20:00 UTC) #18
joone
4 years, 6 months ago (2013-04-23 22:38:35 UTC) #19
Message was sent while issue was closed.
My first patch has finally landed. o/
@James @Elliot @Ben @Avi Thanks for helping review and commit my patch
@jam I see, I will add one reviewer next time.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 81bcdb8aa