Implement STYLE_LOWERCASE style for Aura NativeTextfield.

ui/views/controls/textfield/native_textfield_views.cc

Index: ui/views/controls/textfield/native_textfield_views.cc
diff --git a/ui/views/controls/textfield/native_textfield_views.cc b/ui/views/controls/textfield/native_textfield_views.cc
index 8da7db6bf39db3ba702cd835aaffa34d8c5273d5..53e70b4c515d80b0e8c09557c05a87542efae8e1 100644
--- a/ui/views/controls/textfield/native_textfield_views.cc
+++ b/ui/views/controls/textfield/native_textfield_views.cc
@@ -9,6 +9,7 @@
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/debug/trace_event.h"
+#include "base/i18n/case_conversion.h"
 #include "base/logging.h"
 #include "base/message_loop.h"
 #include "base/utf_string_conversions.h"
@@ -35,6 +36,7 @@
 #include "ui/views/metrics.h"
 #include "ui/views/views_delegate.h"
 #include "ui/views/widget/widget.h"
+#include "unicode/uchar.h"
 
 #if defined(OS_LINUX)
 #include "ui/gfx/gtk_util.h"
@@ -76,9 +78,6 @@ NativeTextfieldViews::NativeTextfieldViews(Textfield* parent)
           TouchSelectionController::create(this))) {
   set_border(text_border_);
 
-  // Lowercase is not supported.
-  DCHECK_NE(parent->style(), Textfield::STYLE_LOWERCASE);
-
 #if defined(OS_CHROMEOS)
   GetRenderText()->SetFontList(gfx::FontList(l10n_util::GetStringUTF8(
       IDS_UI_FONT_FAMILY_CROS)));
@@ -191,6 +190,8 @@ int NativeTextfieldViews::OnPerformDrop(const DropTargetEvent& event) {
       GetRenderText()->FindCursorPosition(event.location());
   string16 text;
   event.data().GetString(&text);
+  if (textfield_->style() & Textfield::STYLE_LOWERCASE)
+    text = base::i18n::ToLower(text);

[xji, 2012/02/17 22:38:24]

I think we should do language specific case mapping. ToLower() is not. Please
refer to http://userguide.icu-project.org/transforms/casemappings

[kochi, 2012/02/23 10:49:07]

According to the header file,
ToLower uses UnicodeString::toLower(), which uses the default locale of ICU. I
thought the default locale of ICU
and Chrome's app locale are synced, so this is language-specific, isn't it?

(Maybe I'm still wrong, I'm new to ICU)

On 2012/02/17 22:38:24, xji wrote:

[xji, 2012/02/23 21:59:17]

You are right! sorry for the false alarm.

 
   // We'll delete the current selection for a drag and drop within this view.
   bool move = initiating_drag_ && !event.IsControlDown() &&
@@ -330,7 +331,10 @@ string16 NativeTextfieldViews::GetText() const {
 }
 
 void NativeTextfieldViews::UpdateText() {
-  model_->SetText(textfield_->text());
+  string16 text = textfield_->text();
+  if (textfield_->style() & Textfield::STYLE_LOWERCASE)
+    text = base::i18n::ToLower(text);
+  model_->SetText(text);
   OnCaretBoundsChanged();
   SchedulePaint();
   textfield_->GetWidget()->NotifyAccessibilityEvent(
@@ -340,7 +344,10 @@ void NativeTextfieldViews::UpdateText() {
 void NativeTextfieldViews::AppendText(const string16& text) {
   if (text.empty())
     return;
-  model_->Append(text);
+  if (textfield_->style() & Textfield::STYLE_LOWERCASE)
+    model_->Append(base::i18n::ToLower(text));
+  else
+    model_->Append(text);
   OnCaretBoundsChanged();
   SchedulePaint();
 }
@@ -674,10 +681,15 @@ void NativeTextfieldViews::InsertText(const string16& text) {
 
   OnBeforeUserAction();
   skip_input_method_cancel_composition_ = true;
+
+  string16 new_text = text;
+  if (textfield_->style() & Textfield::STYLE_LOWERCASE)
+    new_text = base::i18n::ToLower(new_text);

[xji, 2012/02/17 22:38:24]

nit: string16 new_text = (textfield_->style() & Textfield::STYLE_LOWERCASE) ?
                                       base::i18n::ToLower(text) : text;

[msw, 2012/02/18 00:32:38]

why not just reassign to text? new_text seems redundant:
if (textfield_->style() & Textfield::STYLE_LOWERCASE)
    text = base::i18n::ToLower(text);

[xji, 2012/02/18 01:33:38]

|text| is a 'const'.

[kochi, 2012/02/23 10:49:07]

Done.

[kochi, 2012/02/23 10:49:07]

Yes, that's right.

On 2012/02/18 01:33:38, xji wrote:

+
   if (GetRenderText()->insert_mode())
-    model_->InsertText(text);
+    model_->InsertText(new_text);
   else
-    model_->ReplaceText(text);
+    model_->ReplaceText(new_text);
   skip_input_method_cancel_composition_ = false;
   UpdateAfterChange(true, true);
   OnAfterUserAction();
@@ -689,6 +701,9 @@ void NativeTextfieldViews::InsertChar(char16 ch, int flags) {
     return;
   }
 
+  if (textfield_->style() & Textfield::STYLE_LOWERCASE)
+    ch = u_tolower(ch);

[msw, 2012/02/18 00:32:38]

You should ensure this does the language specific case mapping that xji
suggests.

[kochi, 2012/02/23 10:49:07]

This one doesn't seem to handle language-specific
case conversion, and there seems to be no corresponding
API variant for one-character.  Let me look into this further.

+
   OnBeforeUserAction();
   skip_input_method_cancel_composition_ = true;
   if (GetRenderText()->insert_mode())
@@ -1043,6 +1058,12 @@ bool NativeTextfieldViews::Copy() {
 bool NativeTextfieldViews::Paste() {
   const bool success = model_->Paste();
 
+  if (textfield_->style() & Textfield::STYLE_LOWERCASE) {
+    string16 text = model_->GetText();
+    text = base::i18n::ToLower(text);
+    model_->SetText(text);
+  }

[xji, 2012/02/17 22:38:24]

this should not be needed since model_->GetText() should be lower case.

[kochi, 2012/02/23 10:49:07]

The model directly gets the text from clipboard
in the model_->Paste() call above, and modifies the text
internally.  So at this point the string may contain
lower case characters.

[xji, 2012/02/23 21:59:17]

Ah, I see. I thought RenderText will always keep a lower-case text, which is
actually not true (at least on this Paste() operation). I do not like this
inconsistency, but I can not think of a better simple/clean solution regarding
to where to do all the case mappings (not sure whether Textfield is an option).

[kochi, 2012/04/09 06:34:20]

OK, I agree this is not intuitive to understand.
Added a comment.

On 2012/02/23 21:59:17, xji wrote:

+
   // Calls TextfieldController::ContentsChanged() explicitly if the paste action
   // did not change the content at all. See http://crbug.com/79002
   if (success && GetText() == textfield_->text()) {

[xji, 2012/02/17 22:38:24]

In case of STYLE_LOWERCASE, GetText() is lower-cased, but not
textfeild_->text().

[kochi, 2012/02/23 10:49:07]

Right, will fix this.

[kochi, 2012/04/09 06:34:20]

Ah, no.

GetText() in this class is just does model_->GetText(),
so the one in 1062 should be GetText().

And now model_ should have lower cased string, this code
is fine.

On 2012/02/23 10:49:07, Takayoshi Kochi wrote:

[xji, 2012/04/10 22:53:27]

GetText() is lower-cased.
But isn't textfield_->text() the original text, which might be upper-case
letters?


On 2012/04/09 06:34:20, Takayoshi Kochi wrote:

[kochi, 2012/04/11 01:56:41]

I see, sorry for misunderstanding again.
Done.

On 2012/04/10 22:53:27, xji wrote: