Issue 3043023: Add remaining stuff for Mozc configuration DOMUI.

Issue 3043023: Add remaining stuff for Mozc configuration DOMUI. (Closed)

Created:
10 years, 5 months ago by kochi

Modified:
9 years, 7 months ago

Reviewers:
arv (Not doing code reviews), satorux1

CC:
chromium-reviews, dhg, arv (Not doing code reviews), davemoore+watch_chromium.org, ben+cc_chromium.org, mazda, Yusuke Sato

Base URL:
http://src.chromium.org/git/chromium.git

Visibility:
Public.

Description

Add remaining stuff for Mozc configuration DOMUI. This still lacks "Reset to default" button which original configuration dialog has, but all configuration items are implemented. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=53918

Patch Set 1 #

Patch Set 2 : Sync with upstream. #

Total comments: 4

Patch Set 3 : add "#pragma once". #

Patch Set 4 : Fix for review comments. #

Patch Set 5 : Fix for review comments (real for this time) #

Created: 10 years, 4 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+131 lines, -31 lines)			Patch
M	chrome/browser/chromeos/dom_ui/language_chewing_options_handler.cc	View		2 chunks	+1 line, -29 lines	0 comments	Download
M	chrome/browser/chromeos/dom_ui/language_mozc_options_handler.cc	View	1	1 chunk	+10 lines, -0 lines	0 comments	Download
M	chrome/browser/chromeos/dom_ui/language_options_util.h	View	1 2 3 4	2 chunks	+11 lines, -0 lines	0 comments	Download
A	chrome/browser/chromeos/dom_ui/language_options_util.cc	View		1 chunk	+36 lines, -0 lines	0 comments	Download
M	chrome/browser/resources/options/chromeos_language_mozc_options.html	View	1 2 3 4	2 chunks	+70 lines, -1 line	0 comments	Download
M	chrome/chrome_browser.gypi	View	1	1 chunk	+2 lines, -0 lines	0 comments	Download
M	chrome/common/pref_names.cc	View	1	1 chunk	+1 line, -1 line	0 comments	Download

Messages

Total messages: 4 (0 generated)

Expand Messages | Collapse Messages

kochi

Hi Erik, This is the remaining part of the Mozc configuration DOM UI. Could you ...

10 years, 4 months ago (2010-07-28 06:07:39 UTC) #1

satorux1

The title does not match the description. Please change it to "Add remaining stuff for ...

10 years, 4 months ago (2010-07-28 06:15:20 UTC) #2

arv (Not doing code reviews)

LGTM http://codereview.chromium.org/3043023/diff/2001/3004 File chrome/browser/chromeos/dom_ui/language_options_util.h (right): http://codereview.chromium.org/3043023/diff/2001/3004#newcode9 chrome/browser/chromeos/dom_ui/language_options_util.h:9: #include "base/values.h" You don't need to include this ...

10 years, 4 months ago (2010-07-28 06:24:45 UTC) #3

kochi

10 years, 4 months ago (2010-07-28 06:56:02 UTC) #4

Thanks for your quick review!

I've fixed the points you commented.

http://codereview.chromium.org/3043023/diff/2001/3004
File chrome/browser/chromeos/dom_ui/language_options_util.h (right):

http://codereview.chromium.org/3043023/diff/2001/3004#newcode9
chrome/browser/chromeos/dom_ui/language_options_util.h:9: #include
"base/values.h"
On 2010/07/28 06:24:46, arv wrote:
> You don't need to include this here. Just forward declare it like this
> 
> class ListValue;
> 
> 

Done.

http://codereview.chromium.org/3043023/diff/2001/3005
File chrome/browser/resources/options/chromeos_language_mozc_options.html
(right):

http://codereview.chromium.org/3043023/diff/2001/3005#newcode102
chrome/browser/resources/options/chromeos_language_mozc_options.html:102: <td
class="option-name" i18n-content="space_character_formContent"></td>
On 2010/07/28 06:24:46, arv wrote:
> long line here and in a few more places below

Done.

Expand Messages | Collapse Messages