Skip to content

Commit

Permalink
Views Textfield fixes and cleanup.
Browse files Browse the repository at this point in the history
Using STYLE_OBSCURED in the ctor broke with r244436.
Remove 'styles', that ctor, and IsObscured/SetObscured.
Use TextInputType for password textfield configuration.
(rename textfield 'obscured' identifiers to 'password')

Inline STYLE_LOWERCASE to the EditSearchEngineDialog.
Remove GetTextForDisplay and its related handling.

Inline set_controller; remove GetController.
Cleanup users and unit tests.

BUG=131660,334252
TEST=Password textfields work (show '*'s, reveal last typed char, toggle in wifi/wimax dialogs, etc.) as expected. The EditSearchEngineDialog's keyword textfield holds lowercase text as expected.
R=sky@chromium.org
TBR=pfeldman@chromium.org

Review URL: https://codereview.chromium.org/138363004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@245243 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
msw@chromium.org committed Jan 16, 2014
1 parent 7f0dd7f commit 346c44a
Show file tree
Hide file tree
Showing 30 changed files with 190 additions and 369 deletions.
3 changes: 2 additions & 1 deletion chrome/browser/chromeos/options/passphrase_textfield.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,10 @@
namespace chromeos {

PassphraseTextfield::PassphraseTextfield()
: Textfield(views::Textfield::STYLE_OBSCURED),
: Textfield(),
show_fake_(false),
changed_(true) {
SetTextInputType(ui::TEXT_INPUT_TYPE_PASSWORD);
}

void PassphraseTextfield::SetShowFake(bool show_fake) {
Expand Down
24 changes: 12 additions & 12 deletions chrome/browser/chromeos/options/vpn_config_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -529,8 +529,8 @@ void VPNConfigView::Init() {
new views::Label(l10n_util::GetStringUTF16(
IDS_OPTIONS_SETTINGS_INTERNET_OPTIONS_VPN_SERVER_HOSTNAME));
layout_->AddView(server_label);
server_textfield_ = new views::Textfield(views::Textfield::STYLE_DEFAULT);
server_textfield_->SetController(this);
server_textfield_ = new views::Textfield();
server_textfield_->set_controller(this);
layout_->AddView(server_textfield_);
layout_->AddPaddingRow(0, views::kRelatedControlVerticalSpacing);
if (!service_path_.empty()) {
Expand All @@ -543,8 +543,8 @@ void VPNConfigView::Init() {
layout_->AddView(new views::Label(l10n_util::GetStringUTF16(
IDS_OPTIONS_SETTINGS_INTERNET_OPTIONS_VPN_SERVICE_NAME)));
if (service_path_.empty()) {
service_textfield_ = new views::Textfield(views::Textfield::STYLE_DEFAULT);
service_textfield_->SetController(this);
service_textfield_ = new views::Textfield();
service_textfield_->set_controller(this);
layout_->AddView(service_textfield_);
service_text_ = NULL;
} else {
Expand Down Expand Up @@ -581,7 +581,7 @@ void VPNConfigView::Init() {
IDS_OPTIONS_SETTINGS_INTERNET_OPTIONS_VPN_PSK_PASSPHRASE));
layout_->AddView(psk_passphrase_label_);
psk_passphrase_textfield_ = new PassphraseTextfield();
psk_passphrase_textfield_->SetController(this);
psk_passphrase_textfield_->set_controller(this);
layout_->AddView(psk_passphrase_textfield_);
layout_->AddView(
new ControlledSettingIndicatorView(psk_passphrase_ui_data_));
Expand Down Expand Up @@ -622,8 +622,8 @@ void VPNConfigView::Init() {
layout_->StartRow(0, 0);
layout_->AddView(new views::Label(l10n_util::GetStringUTF16(
IDS_OPTIONS_SETTINGS_INTERNET_OPTIONS_VPN_USERNAME)));
username_textfield_ = new views::Textfield(views::Textfield::STYLE_DEFAULT);
username_textfield_->SetController(this);
username_textfield_ = new views::Textfield();
username_textfield_->set_controller(this);
username_textfield_->SetEnabled(username_ui_data_.IsEditable());
layout_->AddView(username_textfield_);
layout_->AddView(new ControlledSettingIndicatorView(username_ui_data_));
Expand All @@ -634,7 +634,7 @@ void VPNConfigView::Init() {
layout_->AddView(new views::Label(l10n_util::GetStringUTF16(
IDS_OPTIONS_SETTINGS_INTERNET_OPTIONS_VPN_USER_PASSPHRASE)));
user_passphrase_textfield_ = new PassphraseTextfield();
user_passphrase_textfield_->SetController(this);
user_passphrase_textfield_->set_controller(this);
user_passphrase_textfield_->SetEnabled(user_passphrase_ui_data_.IsEditable());
layout_->AddView(user_passphrase_textfield_);
layout_->AddView(
Expand All @@ -646,8 +646,8 @@ void VPNConfigView::Init() {
otp_label_ = new views::Label(l10n_util::GetStringUTF16(
IDS_OPTIONS_SETTINGS_INTERNET_OPTIONS_VPN_OTP));
layout_->AddView(otp_label_);
otp_textfield_ = new views::Textfield(views::Textfield::STYLE_DEFAULT);
otp_textfield_->SetController(this);
otp_textfield_ = new views::Textfield();
otp_textfield_->set_controller(this);
layout_->AddView(otp_textfield_);
layout_->AddPaddingRow(0, views::kRelatedControlVerticalSpacing);

Expand All @@ -657,8 +657,8 @@ void VPNConfigView::Init() {
IDS_OPTIONS_SETTINGS_INTERNET_OPTIONS_VPN_GROUP_NAME));
layout_->AddView(group_name_label_);
group_name_textfield_ =
new views::Textfield(views::Textfield::STYLE_DEFAULT);
group_name_textfield_->SetController(this);
new views::Textfield();
group_name_textfield_->set_controller(this);
layout_->AddView(group_name_textfield_);
layout_->AddView(new ControlledSettingIndicatorView(group_name_ui_data_));
layout_->AddPaddingRow(0, views::kRelatedControlVerticalSpacing);
Expand Down
32 changes: 16 additions & 16 deletions chrome/browser/chromeos/options/wifi_config_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -611,11 +611,13 @@ bool WifiConfigView::HandleKeyEvent(views::Textfield* sender,

void WifiConfigView::ButtonPressed(views::Button* sender,
const ui::Event& event) {
if (sender == passphrase_visible_button_) {
if (passphrase_textfield_) {
passphrase_textfield_->SetObscured(!passphrase_textfield_->IsObscured());
passphrase_visible_button_->SetToggled(
!passphrase_textfield_->IsObscured());
if (sender == passphrase_visible_button_ && passphrase_textfield_) {
if (passphrase_textfield_->GetTextInputType() == ui::TEXT_INPUT_TYPE_TEXT) {
passphrase_textfield_->SetTextInputType(ui::TEXT_INPUT_TYPE_PASSWORD);
passphrase_visible_button_->SetToggled(false);
} else {
passphrase_textfield_->SetTextInputType(ui::TEXT_INPUT_TYPE_TEXT);
passphrase_visible_button_->SetToggled(true);
}
} else {
NOTREACHED();
Expand Down Expand Up @@ -936,8 +938,8 @@ void WifiConfigView::Init(bool show_8021x) {
layout->AddView(new views::Label(l10n_util::GetStringUTF16(
IDS_OPTIONS_SETTINGS_INTERNET_OPTIONS_NETWORK_ID)));
if (!wifi) {
ssid_textfield_ = new views::Textfield(views::Textfield::STYLE_DEFAULT);
ssid_textfield_->SetController(this);
ssid_textfield_ = new views::Textfield();
ssid_textfield_->set_controller(this);
ssid_textfield_->SetAccessibleName(l10n_util::GetStringUTF16(
IDS_OPTIONS_SETTINGS_INTERNET_OPTIONS_NETWORK_ID));
layout->AddView(ssid_textfield_);
Expand Down Expand Up @@ -1026,10 +1028,9 @@ void WifiConfigView::Init(bool show_8021x) {
IDS_OPTIONS_SETTINGS_INTERNET_OPTIONS_EAP_SUBJECT_MATCH);
subject_match_label_ = new views::Label(subject_match_label_text);
layout->AddView(subject_match_label_);
subject_match_textfield_ =
new views::Textfield(views::Textfield::STYLE_DEFAULT);
subject_match_textfield_ = new views::Textfield();
subject_match_textfield_->SetAccessibleName(subject_match_label_text);
subject_match_textfield_->SetController(this);
subject_match_textfield_->set_controller(this);
layout->AddView(subject_match_textfield_);
layout->AddPaddingRow(0, views::kRelatedControlVerticalSpacing);

Expand All @@ -1055,9 +1056,9 @@ void WifiConfigView::Init(bool show_8021x) {
IDS_OPTIONS_SETTINGS_INTERNET_OPTIONS_CERT_IDENTITY);
identity_label_ = new views::Label(identity_label_text);
layout->AddView(identity_label_);
identity_textfield_ = new views::Textfield(views::Textfield::STYLE_DEFAULT);
identity_textfield_ = new views::Textfield();
identity_textfield_->SetAccessibleName(identity_label_text);
identity_textfield_->SetController(this);
identity_textfield_->set_controller(this);
identity_textfield_->SetEnabled(identity_ui_data_.IsEditable());
layout->AddView(identity_textfield_);
layout->AddView(new ControlledSettingIndicatorView(identity_ui_data_));
Expand All @@ -1072,7 +1073,7 @@ void WifiConfigView::Init(bool show_8021x) {
passphrase_label_ = new views::Label(passphrase_label_text);
layout->AddView(passphrase_label_);
passphrase_textfield_ = new PassphraseTextfield();
passphrase_textfield_->SetController(this);
passphrase_textfield_->set_controller(this);
// Disable passphrase input initially for other network.
passphrase_label_->SetEnabled(wifi);
passphrase_textfield_->SetEnabled(wifi && passphrase_ui_data_.IsEditable());
Expand Down Expand Up @@ -1121,11 +1122,10 @@ void WifiConfigView::Init(bool show_8021x) {
new views::Label(l10n_util::GetStringUTF16(
IDS_OPTIONS_SETTINGS_INTERNET_OPTIONS_CERT_IDENTITY_ANONYMOUS));
layout->AddView(identity_anonymous_label_);
identity_anonymous_textfield_ = new views::Textfield(
views::Textfield::STYLE_DEFAULT);
identity_anonymous_textfield_ = new views::Textfield();
identity_anonymous_label_->SetEnabled(false);
identity_anonymous_textfield_->SetEnabled(false);
identity_anonymous_textfield_->SetController(this);
identity_anonymous_textfield_->set_controller(this);
layout->AddView(identity_anonymous_textfield_);
layout->AddView(
new ControlledSettingIndicatorView(identity_anonymous_ui_data_));
Expand Down
23 changes: 12 additions & 11 deletions chrome/browser/chromeos/options/wimax_config_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -129,11 +129,13 @@ bool WimaxConfigView::HandleKeyEvent(views::Textfield* sender,

void WimaxConfigView::ButtonPressed(views::Button* sender,
const ui::Event& event) {
if (sender == passphrase_visible_button_) {
if (passphrase_textfield_) {
passphrase_textfield_->SetObscured(!passphrase_textfield_->IsObscured());
passphrase_visible_button_->SetToggled(
!passphrase_textfield_->IsObscured());
if (sender == passphrase_visible_button_ && passphrase_textfield_) {
if (passphrase_textfield_->GetTextInputType() == ui::TEXT_INPUT_TYPE_TEXT) {
passphrase_textfield_->SetTextInputType(ui::TEXT_INPUT_TYPE_PASSWORD);
passphrase_visible_button_->SetToggled(false);
} else {
passphrase_textfield_->SetTextInputType(ui::TEXT_INPUT_TYPE_TEXT);
passphrase_visible_button_->SetToggled(true);
}
} else {
NOTREACHED();
Expand Down Expand Up @@ -242,10 +244,9 @@ void WimaxConfigView::Init() {
IDS_OPTIONS_SETTINGS_INTERNET_OPTIONS_CERT_IDENTITY);
identity_label_ = new views::Label(identity_label_text);
layout->AddView(identity_label_);
identity_textfield_ = new views::Textfield(
views::Textfield::STYLE_DEFAULT);
identity_textfield_ = new views::Textfield();
identity_textfield_->SetAccessibleName(identity_label_text);
identity_textfield_->SetController(this);
identity_textfield_->set_controller(this);
identity_textfield_->SetEnabled(identity_ui_data_.IsEditable());
layout->AddView(identity_textfield_);
layout->AddView(new ControlledSettingIndicatorView(identity_ui_data_));
Expand All @@ -257,9 +258,9 @@ void WimaxConfigView::Init() {
IDS_OPTIONS_SETTINGS_INTERNET_OPTIONS_PASSPHRASE);
passphrase_label_ = new views::Label(passphrase_label_text);
layout->AddView(passphrase_label_);
passphrase_textfield_ = new views::Textfield(
views::Textfield::STYLE_OBSCURED);
passphrase_textfield_->SetController(this);
passphrase_textfield_ = new views::Textfield();
passphrase_textfield_->SetTextInputType(ui::TEXT_INPUT_TYPE_PASSWORD);
passphrase_textfield_->set_controller(this);
passphrase_label_->SetEnabled(true);
passphrase_textfield_->SetEnabled(passphrase_ui_data_.IsEditable());
passphrase_textfield_->SetAccessibleName(passphrase_label_text);
Expand Down
3 changes: 1 addition & 2 deletions chrome/browser/ui/gtk/edit_search_engine_dialog.cc
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,7 @@

namespace {

// Forces text to lowercase when connected to an editable's "insert-text"
// signal. (Like views Textfield::STYLE_LOWERCASE.)
// Forces lowercase text when connected to an editable's "insert-text" signal.
void LowercaseInsertTextHandler(GtkEditable *editable, const gchar *text,
gint length, gint *position, gpointer data) {
base::string16 original_text = base::UTF8ToUTF16(text);
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/ui/views/autofill/decorated_textfield.cc
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ DecoratedTextfield::DecoratedTextfield(

set_placeholder_text(placeholder);
SetText(default_value);
SetController(controller);
set_controller(controller);
}

DecoratedTextfield::~DecoratedTextfield() {}
Expand Down
4 changes: 2 additions & 2 deletions chrome/browser/ui/views/bookmarks/bookmark_editor_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ void BookmarkEditorView::Init() {
title_tf_->SetAccessibleName(
l10n_util::GetStringUTF16(IDS_BOOKMARK_AX_EDITOR_NAME_LABEL));
title_tf_->SetText(title);
title_tf_->SetController(this);
title_tf_->set_controller(this);

if (show_tree_) {
tree_view_ = new views::TreeView;
Expand Down Expand Up @@ -335,7 +335,7 @@ void BookmarkEditorView::Init() {
PrefService* prefs =
profile_ ? user_prefs::UserPrefs::Get(profile_) : NULL;
url_tf_->SetText(chrome::FormatBookmarkURLForDisplay(url, prefs));
url_tf_->SetController(this);
url_tf_->set_controller(this);
url_tf_->SetAccessibleName(
l10n_util::GetStringUTF16(IDS_BOOKMARK_AX_EDITOR_URL_LABEL));

Expand Down
5 changes: 3 additions & 2 deletions chrome/browser/ui/views/crypto_module_password_dialog_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,9 @@ void CryptoModulePasswordDialogView::Init(const std::string& hostname,
password_label_ = new views::Label(l10n_util::GetStringUTF16(
IDS_CRYPTO_MODULE_AUTH_DIALOG_PASSWORD_FIELD));

password_entry_ = new views::Textfield(views::Textfield::STYLE_OBSCURED);
password_entry_->SetController(this);
password_entry_ = new views::Textfield();
password_entry_->SetTextInputType(ui::TEXT_INPUT_TYPE_PASSWORD);
password_entry_->set_controller(this);

views::GridLayout* layout = views::GridLayout::CreatePanel(this);
SetLayoutManager(layout);
Expand Down
30 changes: 18 additions & 12 deletions chrome/browser/ui/views/edit_search_engine_dialog.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "chrome/browser/ui/views/edit_search_engine_dialog.h"

#include "base/i18n/case_conversion.h"
#include "base/i18n/rtl.h"
#include "base/strings/string_util.h"
#include "base/strings/utf_string_conversions.h"
Expand Down Expand Up @@ -102,6 +103,14 @@ bool EditSearchEngineDialog::Accept() {
void EditSearchEngineDialog::ContentsChanged(
Textfield* sender,
const base::string16& new_contents) {
// Force the keyword text to be lowercase, keep the caret or selection.
if (sender == keyword_tf_ && !sender->HasCompositionText()) {
// TODO(msw): Prevent textfield scrolling with selection model changes here.
const gfx::SelectionModel selection_model = sender->GetSelectionModel();
sender->SetText(base::i18n::ToLower(new_contents));
sender->SelectSelectionModel(selection_model);
}

GetDialogClientView()->UpdateDialogButtons();
UpdateImageViews();
}
Expand All @@ -115,18 +124,17 @@ bool EditSearchEngineDialog::HandleKeyEvent(
void EditSearchEngineDialog::Init() {
// Create the views we'll need.
if (controller_->template_url()) {
title_tf_ = CreateTextfield(controller_->template_url()->short_name(),
false);
keyword_tf_ = CreateTextfield(controller_->template_url()->keyword(), true);
title_tf_ = CreateTextfield(controller_->template_url()->short_name());
keyword_tf_ = CreateTextfield(controller_->template_url()->keyword());
url_tf_ = CreateTextfield(
controller_->template_url()->url_ref().DisplayURL(), false);
controller_->template_url()->url_ref().DisplayURL());
// We don't allow users to edit prepopulate URLs. This is done as
// occasionally we need to update the URL of prepopulated TemplateURLs.
url_tf_->SetReadOnly(controller_->template_url()->prepopulate_id() != 0);
} else {
title_tf_ = CreateTextfield(base::string16(), false);
keyword_tf_ = CreateTextfield(base::string16(), true);
url_tf_ = CreateTextfield(base::string16(), false);
title_tf_ = CreateTextfield(base::string16());
keyword_tf_ = CreateTextfield(base::string16());
url_tf_ = CreateTextfield(base::string16());
}
title_iv_ = new views::ImageView();
keyword_iv_ = new views::ImageView();
Expand Down Expand Up @@ -223,12 +231,10 @@ views::Label* EditSearchEngineDialog::CreateLabel(int message_id) {
return label;
}

Textfield* EditSearchEngineDialog::CreateTextfield(const base::string16& text,
bool lowercase) {
Textfield* text_field = new Textfield(
lowercase ? Textfield::STYLE_LOWERCASE : Textfield::STYLE_DEFAULT);
Textfield* EditSearchEngineDialog::CreateTextfield(const base::string16& text) {
Textfield* text_field = new Textfield();
text_field->SetText(text);
text_field->SetController(this);
text_field->set_controller(this);
return text_field;
}

Expand Down
5 changes: 2 additions & 3 deletions chrome/browser/ui/views/edit_search_engine_dialog.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,8 @@ class EditSearchEngineDialog : public views::TextfieldController,
// Create a Label containing the text with the specified message id.
views::Label* CreateLabel(int message_id);

// Creates a text field with the specified text. If |lowercase| is true, the
// Textfield is configured to map all input to lower case.
views::Textfield* CreateTextfield(const base::string16& text, bool lowercase);
// Creates a text field with the specified text.
views::Textfield* CreateTextfield(const base::string16& text);

// Invokes UpdateImageView for each of the images views.
void UpdateImageViews();
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/ui/views/find_bar_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ FindBarView::FindBarView(FindBarHost* host)
find_text_ = new views::Textfield;
find_text_->set_id(VIEW_ID_FIND_IN_PAGE_TEXT_FIELD);
find_text_->set_default_width_in_chars(kDefaultCharWidth);
find_text_->SetController(this);
find_text_->set_controller(this);
find_text_->SetAccessibleName(l10n_util::GetStringUTF16(IDS_ACCNAME_FIND));
// The find bar textfield has a background image instead of a border.
const gfx::Insets insets = find_text_->GetInsets();
Expand Down
5 changes: 3 additions & 2 deletions chrome/browser/ui/views/login_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,15 @@ using views::GridLayout;

LoginView::LoginView(const base::string16& explanation,
LoginModel* model)
: username_field_(new views::Textfield),
password_field_(new views::Textfield(views::Textfield::STYLE_OBSCURED)),
: username_field_(new views::Textfield()),
password_field_(new views::Textfield()),
username_label_(new views::Label(
l10n_util::GetStringUTF16(IDS_LOGIN_DIALOG_USERNAME_FIELD))),
password_label_(new views::Label(
l10n_util::GetStringUTF16(IDS_LOGIN_DIALOG_PASSWORD_FIELD))),
message_label_(new views::Label(explanation)),
login_model_(model) {
password_field_->SetTextInputType(ui::TEXT_INPUT_TYPE_PASSWORD);
message_label_->SetMultiLine(true);
message_label_->SetHorizontalAlignment(gfx::ALIGN_LEFT);
message_label_->SetAllowCharacterBreak(true);
Expand Down
6 changes: 2 additions & 4 deletions chrome/browser/ui/views/login_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,8 @@ class LoginModel;
class LoginView : public views::View, public LoginModelObserver {
public:
// |model| is observed for the entire lifetime of the LoginView.
// Therefore |model| should not be destroyed before the LoginView
// object.
LoginView(const base::string16& explanation,
LoginModel* model);
// Therefore |model| should not be destroyed before the LoginView object.
LoginView(const base::string16& explanation, LoginModel* model);
virtual ~LoginView();

// Access the data in the username/password text fields.
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/ui/views/omnibox/omnibox_view_views.cc
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ OmniboxViewViews::~OmniboxViewViews() {
// OmniboxViewViews public:

void OmniboxViewViews::Init() {
SetController(this);
set_controller(this);
SetTextInputType(DetermineTextInputType());
SetBackgroundColor(location_bar_view_->GetColor(
ToolbarModel::NONE, LocationBarView::BACKGROUND));
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/ui/views/password_generation_bubble_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ void PasswordGenerationBubbleView::Init() {
textfield_ = new views::Textfield();
textfield_->set_default_width_in_chars(kDefaultTextFieldChars);
textfield_->SetText(base::ASCIIToUTF16(password_generator_->Generate()));
textfield_->SetController(this);
textfield_->set_controller(this);

textfield_wrapper_ = new TextfieldWrapper(textfield_,
regenerate_button_);
Expand Down
Loading

0 comments on commit 346c44a

Please sign in to comment.