Skip to content

Commit

Permalink
[Passwords] Add strong label password experiment for generation
Browse files Browse the repository at this point in the history
* Allows displaying "Strong password" label in password input forms
* The label starts being displayed whenever user hovers over "suggest
  strong password", is still displayed when the user accepts suggested
  password
* The label is cleared when the user stops hovering over the "suggest
  strong password" or when they edit it after accepting

The functionality is protected by a new runtime feature flag which
generates corresponding Finch feature flag that will be later enabled
for a 1% experiment on Stable (not intended to fully launch).

Mock: https://screenshot.googleplex.com/8eEovnoFLP8nHqt
Screenshot: https://screenshot.googleplex.com/9SifkDDZmi5qXHV

Bug: 1444073
Change-Id: I22a4eb22fa1ab65ece6ac589d1853ce0d96b2f98
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4874256
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Commit-Queue: Rafał Godlewski <rgod@google.com>
Cr-Commit-Position: refs/heads/main@{#1211349}
  • Loading branch information
rgod authored and Chromium LUCI CQ committed Oct 18, 2023
1 parent 9d9929c commit 2302f59
Show file tree
Hide file tree
Showing 13 changed files with 134 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
#include "services/service_manager/public/cpp/interface_provider.h"
#include "third_party/blink/public/common/associated_interfaces/associated_interface_provider.h"
#include "third_party/blink/public/common/features.h"
#include "third_party/blink/public/common/features_generated.h"
#include "third_party/blink/public/common/input/web_input_event.h"
#include "third_party/blink/public/platform/web_security_origin.h"
#include "third_party/blink/public/platform/web_vector.h"
Expand Down Expand Up @@ -1771,6 +1772,9 @@ void PasswordAutofillAgent::ClearPreview(WebInputElement* username,
base::checked_cast<unsigned>(username->Value().length()));
}
if (!password->IsNull() && !password->SuggestedValue().IsEmpty()) {
if (base::FeatureList::IsEnabled(blink::features::kPasswordStrongLabel)) {
password->SetShouldShowStrongPasswordLabel(false);
}
password->SetSuggestedValue(WebString());
password->SetAutofillState(password_autofill_state_);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include "google_apis/gaia/gaia_urls.h"
#include "third_party/blink/public/common/associated_interfaces/associated_interface_provider.h"
#include "third_party/blink/public/common/features.h"
#include "third_party/blink/public/common/features_generated.h"
#include "third_party/blink/public/platform/web_security_origin.h"
#include "third_party/blink/public/platform/web_vector.h"
#include "third_party/blink/public/web/web_document.h"
Expand Down Expand Up @@ -84,11 +85,17 @@ void CopyElementValueToOtherInputElements(

void PreviewGeneratedValue(WebInputElement& input_element,
const blink::WebString& value) {
if (base::FeatureList::IsEnabled(blink::features::kPasswordStrongLabel)) {
input_element.SetShouldShowStrongPasswordLabel(true);
}
input_element.SetShouldRevealPassword(true);
input_element.SetSuggestedValue(value);
}

void ClearPreviewedValue(WebInputElement& input_element) {
if (base::FeatureList::IsEnabled(blink::features::kPasswordStrongLabel)) {
input_element.SetShouldShowStrongPasswordLabel(false);
}
input_element.SetShouldRevealPassword(false);
input_element.SetSuggestedValue(blink::WebString());
}
Expand Down
6 changes: 6 additions & 0 deletions third_party/blink/public/web/web_input_element.h
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,12 @@ class BLINK_EXPORT WebInputElement final : public WebFormControlElement {
// Returns true if the text of the element should be visible.
bool ShouldRevealPassword() const;

// If true, forces "Strong Password" label to be visible in the field.
void SetShouldShowStrongPasswordLabel(bool value);

// Returns whether "Strong Password" label should be visible in the field.
bool ShouldShowStrongPasswordLabel() const;

#if BUILDFLAG(IS_ANDROID)
// Returns whether this is the last element within its form.
bool IsLastInputElementInForm();
Expand Down
8 changes: 8 additions & 0 deletions third_party/blink/renderer/core/exported/web_input_element.cc
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,14 @@ bool WebInputElement::ShouldRevealPassword() const {
return ConstUnwrap<HTMLInputElement>()->ShouldRevealPassword();
}

void WebInputElement::SetShouldShowStrongPasswordLabel(bool value) {
Unwrap<HTMLInputElement>()->SetShouldShowStrongPasswordLabel(value);
}

bool WebInputElement::ShouldShowStrongPasswordLabel() const {
return ConstUnwrap<HTMLInputElement>()->ShouldShowStrongPasswordLabel();
}

#if BUILDFLAG(IS_ANDROID)
bool WebInputElement::IsLastInputElementInForm() {
return Unwrap<HTMLInputElement>()->IsLastInputElementInForm();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,12 @@ class CORE_EXPORT HTMLInputElement
bool ShouldDrawCapsLockIndicator() const;
void SetShouldRevealPassword(bool value);
bool ShouldRevealPassword() const { return should_reveal_password_; }
void SetShouldShowStrongPasswordLabel(bool value) {
should_show_strong_password_label_ = value;
}
bool ShouldShowStrongPasswordLabel() const {
return should_show_strong_password_label_;
}
#if BUILDFLAG(IS_ANDROID)
bool IsLastInputElementInForm();
void DispatchSimulatedEnter();
Expand Down Expand Up @@ -498,6 +504,7 @@ class CORE_EXPORT HTMLInputElement
unsigned needs_to_update_view_value_ : 1;
unsigned is_placeholder_visible_ : 1;
unsigned has_been_password_field_ : 1;
unsigned should_show_strong_password_label_ : 1;
Member<InputType> input_type_;
Member<InputTypeView> input_type_view_;
// The ImageLoader must be owned by this element because the loader code
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
#include "third_party/blink/renderer/core/html/shadow/shadow_element_names.h"
#include "third_party/blink/renderer/core/input/keyboard_event_manager.h"
#include "third_party/blink/renderer/core/input_type_names.h"
#include "third_party/blink/renderer/platform/runtime_enabled_features.h"

namespace blink {

Expand Down Expand Up @@ -75,7 +76,8 @@ bool PasswordInputType::ShouldRespectListAttribute() {
}

bool PasswordInputType::NeedsContainer() const {
return RuntimeEnabledFeatures::PasswordRevealEnabled();
return RuntimeEnabledFeatures::PasswordRevealEnabled() ||
RuntimeEnabledFeatures::PasswordStrongLabelEnabled();
}

void PasswordInputType::CreateShadowSubtree() {
Expand All @@ -91,6 +93,17 @@ void PasswordInputType::CreateShadowSubtree() {
GetElement().GetDocument()),
view_port->nextSibling());
}

if (RuntimeEnabledFeatures::PasswordStrongLabelEnabled()) {
Element* container = ContainerElement();
Element* view_port = GetElement().UserAgentShadowRoot()->getElementById(
shadow_element_names::kIdEditingViewPort);
DCHECK(container);
DCHECK(view_port);
container->InsertBefore(MakeGarbageCollected<PasswordStrongLabelElement>(
GetElement().GetDocument()),
view_port->nextSibling());
}
}

void PasswordInputType::DidSetValueByUserEdit() {
Expand All @@ -101,6 +114,12 @@ void PasswordInputType::DidSetValueByUserEdit() {
}
UpdatePasswordRevealButton();
}
if (RuntimeEnabledFeatures::PasswordStrongLabelEnabled()) {
// If any character is edited by the user, we no longer display the label.
GetElement().SetShouldShowStrongPasswordLabel(false);
UpdateStrongPasswordLabel();
}

BaseTextInputType::DidSetValueByUserEdit();
}

Expand All @@ -112,6 +131,10 @@ void PasswordInputType::DidSetValue(const String& string, bool value_changed) {
UpdatePasswordRevealButton();
}
}
if (RuntimeEnabledFeatures::PasswordStrongLabelEnabled() && value_changed) {
UpdateStrongPasswordLabel();
}

BaseTextInputType::DidSetValue(string, value_changed);
}

Expand All @@ -120,6 +143,10 @@ void PasswordInputType::UpdateView() {

if (RuntimeEnabledFeatures::PasswordRevealEnabled())
UpdatePasswordRevealButton();

if (RuntimeEnabledFeatures::PasswordStrongLabelEnabled()) {
UpdateStrongPasswordLabel();
}
}

void PasswordInputType::CapsLockStateMayHaveChanged() {
Expand Down Expand Up @@ -183,6 +210,16 @@ void PasswordInputType::UpdatePasswordRevealButton() {
}
}

void PasswordInputType::UpdateStrongPasswordLabel() {
Element* label = GetElement().EnsureShadowSubtree()->getElementById(
shadow_element_names::kIdPasswordStrongLabel);
if (GetElement().ShouldShowStrongPasswordLabel()) {
label->RemoveInlineStyleProperty(CSSPropertyID::kDisplay);
} else {
label->SetInlineStyleProperty(CSSPropertyID::kDisplay, CSSValueID::kNone);
}
}

void PasswordInputType::ForwardEvent(Event& event) {
BaseTextInputType::ForwardEvent(event);

Expand All @@ -199,6 +236,10 @@ void PasswordInputType::HandleBlurEvent() {
UpdatePasswordRevealButton();
}

if (RuntimeEnabledFeatures::PasswordStrongLabelEnabled()) {
UpdateStrongPasswordLabel();
}

BaseTextInputType::HandleBlurEvent();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ class PasswordInputType final : public BaseTextInputType {
void CapsLockStateMayHaveChanged() override;
bool ShouldDrawCapsLockIndicator() const override;
void UpdatePasswordRevealButton();
void UpdateStrongPasswordLabel();
void DidSetValueByUserEdit() override;
void DidSetValue(const String&, bool value_changed) override;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -290,4 +290,15 @@ bool PasswordRevealButtonElement::WillRespondToMouseClickEvents() {

return HTMLDivElement::WillRespondToMouseClickEvents();
}

// ----------------------------

PasswordStrongLabelElement::PasswordStrongLabelElement(Document& document)
: HTMLDivElement(document) {
SetShadowPseudoId(AtomicString("-internal-strong"));
setAttribute(html_names::kIdAttr,
shadow_element_names::kIdPasswordStrongLabel);
// TODO(crbug.com/1444073): Add internationalized string.
setTextContent("Strong password");
}
} // namespace blink
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,14 @@ class PasswordRevealButtonElement final : public HTMLDivElement {
bool SupportsFocus() const override { return false; }
};

class PasswordStrongLabelElement final : public HTMLDivElement {
public:
explicit PasswordStrongLabelElement(Document&);

private:
bool SupportsFocus() const override { return false; }
};

} // namespace blink

#endif // THIRD_PARTY_BLINK_RENDERER_CORE_HTML_FORMS_TEXT_CONTROL_INNER_ELEMENTS_H_
6 changes: 6 additions & 0 deletions third_party/blink/renderer/core/html/resources/html.css
Original file line number Diff line number Diff line change
Expand Up @@ -619,6 +619,12 @@ input[type="password" i]::-internal-reveal:focus {
cursor: pointer;
}

input[type="password" i]::-internal-strong {
color: rgb(66, 133, 244);
font-family: roboto;
-webkit-text-security: none;
}

input[type="hidden" i], input[type="image" i], input[type="file" i] {
appearance: none;
padding: initial;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@
name: "password-reveal",
Symbol: "kIdPasswordRevealButton",
},
{
name: "password-strong-label",
Symbol: "kIdPasswordStrongLabel",
},
{
name: "thumb",
Symbol: "kIdSliderThumb",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2848,6 +2848,10 @@
name: "PasswordReveal",
base_feature: "none",
},
{
name: "PasswordStrongLabel",
status: "experimental",
},
{
name: "PastingBlocksSVGUseNonLocalHrefs",
status: "stable",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,19 @@ and whether they should be revealed.
| shadow:pseudoId="-internal-input-suggested"
| "suggested value"
| <div>
| "initial value"
| id="text-field-container"
| pseudo="-webkit-textfield-decoration-container"
| shadow:pseudoId="-webkit-textfield-decoration-container"
| <div>
| id="editing-view-port"
| <div>
| "initial value"
| <div>
| id="password-strong-label"
| pseudo="-internal-strong"
| style="display: none;"
| shadow:pseudoId="-internal-strong"
| "Strong password"
| <input>
| id="password2"
| type="password"
Expand All @@ -28,7 +40,19 @@ and whether they should be revealed.
| shadow:pseudoId="-internal-input-suggested"
| "suggested value"
| <div>
| "initial value"
| id="text-field-container"
| pseudo="-webkit-textfield-decoration-container"
| shadow:pseudoId="-webkit-textfield-decoration-container"
| <div>
| id="editing-view-port"
| <div>
| "initial value"
| <div>
| id="password-strong-label"
| pseudo="-internal-strong"
| style="display: none;"
| shadow:pseudoId="-internal-strong"
| "Strong password"
| "password_with_masked_suggestion.value: initial value"
| "internals.suggestedValue(password_with_masked_suggestion): suggested value"
| "password_with_revealed_suggestion.value: initial value"
Expand Down

0 comments on commit 2302f59

Please sign in to comment.