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

Issue 7713033: Integrate the Spelling service to Chrome. (Closed)

Created:
9 years, 4 months ago by Hironori Bono
Modified:
9 years, 3 months ago
CC:
chromium-reviews, arv (Not doing code reviews), brettw-cc_chromium.org
Visibility:
Public.

Description

Integrate the Spelling service to Chrome. This change integrates the Spelling service (a.k.a. "Did you mean") to a context-menu of Chrome so we can see the spell-check result retrieved from the Spelling service. This change sends a JSON-RPC request to the service in the background and update a context-menu item while showing to prevent junkiness. BUG=29191, 93746 TEST=right-click a misspelled word and see it is updated. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=99633

Patch Set 1 : '' #

Total comments: 38

Patch Set 2 : '' #

Patch Set 3 : '' #

Total comments: 4

Patch Set 4 : '' #

Patch Set 5 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+832 lines, -7 lines) Patch
M chrome/app/chrome_command_ids.h View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M chrome/app/generated_resources.grd View 1 2 3 4 3 chunks +15 lines, -1 line 0 comments Download
M chrome/browser/profiles/profile.cc View 1 2 3 4 1 chunk +7 lines, -0 lines 0 comments Download
M chrome/browser/tab_contents/render_view_context_menu.h View 1 2 3 4 5 chunks +86 lines, -1 line 0 comments Download
M chrome/browser/tab_contents/render_view_context_menu.cc View 1 2 3 4 6 chunks +62 lines, -1 line 0 comments Download
M chrome/browser/tab_contents/render_view_context_menu_gtk.h View 1 2 3 4 1 chunk +5 lines, -0 lines 0 comments Download
M chrome/browser/tab_contents/render_view_context_menu_gtk.cc View 1 2 3 4 3 chunks +68 lines, -0 lines 0 comments Download
M chrome/browser/tab_contents/render_view_context_menu_mac.h View 1 2 3 4 1 chunk +5 lines, -0 lines 0 comments Download
M chrome/browser/tab_contents/render_view_context_menu_mac.mm View 1 2 3 4 2 chunks +43 lines, -0 lines 0 comments Download
A chrome/browser/tab_contents/render_view_context_menu_observer.h View 1 1 chunk +102 lines, -0 lines 0 comments Download
A chrome/browser/tab_contents/render_view_context_menu_observer.cc View 1 chunk +19 lines, -0 lines 0 comments Download
A chrome/browser/tab_contents/spelling_menu_observer.h View 1 1 chunk +106 lines, -0 lines 0 comments Download
A chrome/browser/tab_contents/spelling_menu_observer.cc View 1 2 3 4 1 chunk +276 lines, -0 lines 0 comments Download
M chrome/browser/ui/views/tab_contents/render_view_context_menu_views.h View 1 2 3 4 2 chunks +12 lines, -4 lines 0 comments Download
M chrome/browser/ui/views/tab_contents/render_view_context_menu_views.cc View 1 2 3 4 1 chunk +17 lines, -0 lines 0 comments Download
M chrome/chrome_browser.gypi View 1 2 3 4 2 chunks +4 lines, -0 lines 0 comments Download
M chrome/common/pref_names.h View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M chrome/common/pref_names.cc View 1 2 3 4 1 chunk +3 lines, -0 lines 0 comments Download

Messages

Total messages: 12 (0 generated)
Hironori Bono
Greetings, Unfortunately, this change becomes huge and needs approvals of several reviewers. To describe the ...
9 years, 4 months ago (2011-08-23 10:14:47 UTC) #1
Ben Goodger (Google)
I'm going to let others with more recent experience with the contents world (e.g. Avi) ...
9 years, 4 months ago (2011-08-23 16:12:52 UTC) #2
Avi (use Gerrit)
Your approach looks reasonable. Noted spelling errors, but I have a naming question. http://codereview.chromium.org/7713033/diff/1008/chrome/app/generated_resources.grd File ...
9 years, 4 months ago (2011-08-23 18:38:19 UTC) #3
Miranda Callahan
On 2011/08/23 18:38:19, Avi wrote: > Your approach looks reasonable. Noted spelling errors, but I ...
9 years, 4 months ago (2011-08-23 18:57:49 UTC) #4
Hironori Bono
Greetings Ben, Avi, and Miranda, Thank you so much for your review and comments. (It ...
9 years, 4 months ago (2011-08-25 11:18:58 UTC) #5
Avi (use Gerrit)
http://codereview.chromium.org/7713033/diff/1008/chrome/browser/tab_contents/render_view_context_menu.h File chrome/browser/tab_contents/render_view_context_menu.h (right): http://codereview.chromium.org/7713033/diff/1008/chrome/browser/tab_contents/render_view_context_menu.h#newcode89 chrome/browser/tab_contents/render_view_context_menu.h:89: class RenderViewContextMenuDelegate { That's better. I'm struggling to come ...
9 years, 4 months ago (2011-08-25 14:59:11 UTC) #6
Avi (use Gerrit)
In any case, LGTM from me.
9 years, 4 months ago (2011-08-25 18:20:56 UTC) #7
Hironori Bono
Greetings Avi, Thank you for your review. Even though I have not updated this change, ...
9 years, 3 months ago (2011-08-29 08:17:04 UTC) #8
Avi (use Gerrit)
Well, LGTM with the name you've got now. We're moving pieces of the view into ...
9 years, 3 months ago (2011-08-29 12:54:42 UTC) #9
Ben Goodger (Google)
LGTM for the views btis http://codereview.chromium.org/7713033/diff/20001/chrome/browser/ui/views/tab_contents/render_view_context_menu_views.h File chrome/browser/ui/views/tab_contents/render_view_context_menu_views.h (right): http://codereview.chromium.org/7713033/diff/20001/chrome/browser/ui/views/tab_contents/render_view_context_menu_views.h#newcode37 chrome/browser/ui/views/tab_contents/render_view_context_menu_views.h:37: // RenderViewContextMenuDelegate implementation. Match ...
9 years, 3 months ago (2011-08-30 17:40:35 UTC) #10
Hironori Bono
Greetings Ben, Thank you for your review and comments. I have updated the styles of ...
9 years, 3 months ago (2011-09-01 10:33:05 UTC) #11
csilv
9 years, 3 months ago (2011-09-01 16:42:13 UTC) #12
Options related changes, LGTM.

Powered by Google App Engine
This is Rietveld 408576698