Reintroduce password support to NativeTextfieldViews

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 c2c205427324a4600f0d0066cc40be8586da7c98..f7320ca84a9e77e1c966b04ef4a53b3bfba34d48 100644
--- a/ui/views/controls/textfield/native_textfield_views.cc
+++ b/ui/views/controls/textfield/native_textfield_views.cc
@@ -309,7 +309,8 @@ void NativeTextfieldViews::WriteDragDataForView(views::View* sender,
 
 int NativeTextfieldViews::GetDragOperationsForView(views::View* sender,
                                                    const gfx::Point& p) {
-  if (!textfield_->enabled() || !GetRenderText()->IsPointInSelection(p))
+  if (!textfield_->enabled() || textfield_->IsObscured() ||

[msw, 2012/02/22 00:33:26]

Why disable drag instead of just using GetDisplayText?

[benrg, 2012/02/22 02:25:43]

See below.

+      !GetRenderText()->IsPointInSelection(p))
     return ui::DragDropTypes::DRAG_NONE;
   if (sender == this && !textfield_->read_only())
     return ui::DragDropTypes::DRAG_MOVE | ui::DragDropTypes::DRAG_COPY;
@@ -411,7 +412,7 @@ void NativeTextfieldViews::UpdateFont() {
 }
 
 void NativeTextfieldViews::UpdateIsObscured() {
-  // TODO(benrg): GetRenderText()->SetObscured(textfield_->IsObscured());
+  GetRenderText()->SetObscured(textfield_->IsObscured());
   OnCaretBoundsChanged();
   SchedulePaint();
   OnTextInputTypeChanged();
@@ -552,9 +553,9 @@ bool NativeTextfieldViews::IsCommandIdEnabled(int command_id) const {
   string16 result;
   switch (command_id) {
     case IDS_APP_CUT:
-      return editable && model_->HasSelection();
+      return editable && model_->HasSelection() && !textfield_->IsObscured();

[msw, 2012/02/22 00:33:26]

Why disable cut/copy instead of just using GetDisplayText?
Same Q for key handling and TextfieldViewsModel::Cut|Copy changes.

[benrg, 2012/02/22 02:25:43]

I doubt that a user who cuts/copies/drags text from an obscured textbox wants a
bunch of asterisks (or U+25CF "black circle"s, or whatever we might use in the
future). The asterisks are not real. Personally I would rather allow
cut/copy/drag (extracting the real text), but these are no-ops on other
platforms and I think we agreed to replicate that behavior. Apparently the
threat is an attacker who has access to the keyboard for long enough to
copy-paste the password into Notepad, but not long enough to install a keylogger
or run a window-spy utility.

     case IDS_APP_COPY:
-      return model_->HasSelection();
+      return model_->HasSelection() && !textfield_->IsObscured();
     case IDS_APP_PASTE:
       ViewsDelegate::views_delegate->GetClipboard()
           ->ReadText(ui::Clipboard::BUFFER_STANDARD, &result);
@@ -575,30 +576,28 @@ bool NativeTextfieldViews::GetAcceleratorForCommandId(int command_id,
 
 void NativeTextfieldViews::ExecuteCommand(int command_id) {
   bool text_changed = false;
-  bool editable = !textfield_->read_only();
   OnBeforeUserAction();
-  switch (command_id) {
-    case IDS_APP_CUT:
-      if (editable)
+  if (IsCommandIdEnabled(command_id)) {

[xji, 2012/02/18 01:28:47]

UpdateAfterChange() could be moved inside the 'if' statement.

do we need "OnBeforeUserAction()" and "OnAfterUserAction" if 
"IsCommandIdEnabled() == false"?

+    switch (command_id) {
+      case IDS_APP_CUT:
         text_changed = Cut();
-      break;
-    case IDS_APP_COPY:
-      Copy();
-      break;
-    case IDS_APP_PASTE:
-      if (editable)
+        break;
+      case IDS_APP_COPY:
+        Copy();
+        break;
+      case IDS_APP_PASTE:
         text_changed = Paste();
-      break;
-    case IDS_APP_DELETE:
-      if (editable)
+        break;
+      case IDS_APP_DELETE:
         text_changed = model_->Delete();
-      break;
-    case IDS_APP_SELECT_ALL:
-      SelectAll();
-      break;
-    default:
-      textfield_->GetController()->ExecuteCommand(command_id);
-      break;
+        break;
+      case IDS_APP_SELECT_ALL:
+        SelectAll();
+        break;
+      default:
+        textfield_->GetController()->ExecuteCommand(command_id);
+        break;
+    }
   }
 
   // The cursor must have changed if text changed during cut/paste/delete.
@@ -838,6 +837,7 @@ bool NativeTextfieldViews::HandleKeyEvent(const KeyEvent& key_event) {
 
     OnBeforeUserAction();
     bool editable = !textfield_->read_only();
+    bool readable = !textfield_->IsObscured();
     bool selection = key_event.IsShiftDown();
     bool control = key_event.IsControlDown();
     bool text_changed = false;
@@ -858,11 +858,11 @@ bool NativeTextfieldViews::HandleKeyEvent(const KeyEvent& key_event) {
         }
         break;
       case ui::VKEY_X:
-        if (control && editable)
+        if (control && editable && readable)
           cursor_changed = text_changed = Cut();
         break;
       case ui::VKEY_C:
-        if (control)
+        if (control && readable)
           Copy();
         break;
       case ui::VKEY_V: