Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change auto directionality of hidden, password, and button controls #9689

Merged
merged 4 commits into from
Oct 24, 2023

Conversation

annevk
Copy link
Member

@annevk annevk commented Sep 5, 2023

This makes two changes:

  • Applies the auto directionality algorithm to Hidden, Password, Submit Button, Reset Button, and Button input elements.
  • Adds support for the dirname attribute to Password and Submit Button input elements.

And it uses a shared concept between these two features so they will no longer diverge going forward. (The concept can still be shared despite Reset Button and Button input elements not supporting the dirname attribute as the concept is only consulted in form submission where those elements have already been filtered out.)

Tests: web-platform-tests/wpt#42427

Fixes #3330 and fixes #9669.

(See WHATWG Working Mode: Changes for more details.)


/acknowledgements.html ( diff )
/dom.html ( diff )
/form-control-infrastructure.html ( diff )
/input.html ( diff )

@annevk
Copy link
Member Author

annevk commented Sep 5, 2023

I would appreciate feedback from @jfkthame, @aphillips, @fantasai, and @dbaron.

@annevk annevk added normative change i18n-tracker Group bringing to attention of Internationalization, or tracked by i18n but not needing response. needs tests Moving the issue forward requires someone to write tests labels Sep 5, 2023
Copy link
Contributor

@aphillips aphillips left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@annevk

This comment was marked as duplicate.

Include the input Password state in the auto directionality algorithm to achieve consistency with input elements where the dirname attribute will apply as per the discussion in # #9669.

Also include the input buttons, except for the Image Button. Notably this does not include the button element as there the contents of the element and the value are distinct concepts (value is only used for submission and not for rendering).

Tests: ...

Fixes #3330.
@annevk annevk force-pushed the annevk/direction-of-password-and-buttons branch from 2a822fa to 5cac0d2 Compare October 6, 2023 14:41
@@ -13592,16 +13594,27 @@ Transport Protocol">HTTP&lt;/abbr> today.&lt;/p></code></pre> <!-- DO NOT REWRAP
<span>HTML elements</span>, it cannot be present on elements from other namespaces. Thus, elements
from other namespaces always end up using the <span>parent directionality</span>.</p>

<p>The <dfn>auto-directionality form-associated elements</dfn> are:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked into making this a proper subcategory of form-associated elements, but it's extremely awkward due to the type attribute constraints which categories mostly don't have. It's also very boilerplate heavy. Not sure if this even makes sense for form-associated custom elements, but I'd suggest we wait with an actual subcategory until then.

@annevk
Copy link
Member Author

annevk commented Oct 6, 2023

I have updated this to include the dirname changes. Upon further consideration it seemed better to keep these two in lockstep and I further enshrined that in this PR by making auto directionality and dirname use a shared concept.

source Show resolved Hide resolved
annevk added a commit to web-platform-tests/wpt that referenced this pull request Oct 9, 2023
For whatwg/html#9689.

Helps with #25569 as this area was barely tested.

(Also correct some typos while here.)
@annevk annevk added topic: forms and removed needs tests Moving the issue forward requires someone to write tests labels Oct 9, 2023
@annevk annevk changed the title Change auto directionality of password and button controls Change auto directionality of hidden, password, and button controls Oct 9, 2023
@zcorpan
Copy link
Member

zcorpan commented Oct 9, 2023

Looks ok to me. fyi @vinhill

@nt1m
Copy link
Member

nt1m commented Oct 10, 2023

@annevk Does the auto directionality algorithm on passwords make sense? IMO, it doesn't from the user's perspective since it gives hints about the user's password.

@domenic
Copy link
Member

domenic commented Oct 10, 2023

I don't have any expertise in this area so am happy to turn my review over to @zcorpan.

@annevk
Copy link
Member Author

annevk commented Oct 10, 2023

@nt1m the website can already read out the password, no? A subset of user agents already supported dirname there too, including WebKit. And removing password support from that seemed more risky to me. (This is also discussed in the linked issues.)

@nt1m
Copy link
Member

nt1m commented Oct 10, 2023

@nt1m the website can already read out the password, no? A subset of user agents already supported dirname there too, including WebKit. And removing password support from that seemed more risky to me. (This is also discussed in the linked issues.)

This isn't about the website being able to read the password, but more about changes in directionality being reflected as you type the password, which is a bad UX imo, unless I'm misunderstanding the implications of the dir=auto change.

@annevk
Copy link
Member Author

annevk commented Oct 10, 2023

@nt1m your first comment seemed to be about the website reading it? I don't really see how it's different for Password compared to Text. It seems potentially confusing for both of them, but I don't see why we should make them inconsistent. (These are good reasons to avoid using dir=auto on inputs most of the time though.)

@nt1m
Copy link
Member

nt1m commented Oct 10, 2023

@nt1m your first comment seemed to be about the website reading it? I don't really see how it's different for Password compared to Text. It seems potentially confusing for both of them, but I don't see why we should make them inconsistent. (These are good reasons to avoid using dir=auto on inputs most of the time though.)

Password fields obscure their content. With dir=auto, the user will see bullets shifting from left to right or vice versa, which is confusing and disruptive as you type, but also gives hints at the directionality of the password to anyone looking at the screen.

@fantasai
Copy link
Contributor

fantasai commented Oct 13, 2023

@nt1m If you're looking at the user typing it live, couldn't you guess the directionality from the direction of the cursor movement anyway? If you want to reduce the ability to tell the base directionality once the password is typed, you can set it to text-align: center which doesn't change...

data-x="attr-input-type-tel">Telephone</span>, <span data-x="attr-input-type-url">URL</span>,
or <span data-x="attr-input-type-email">Email</span> state, then:</p>
attribute's value is not the empty string, and the element is an <span
data-x="auto-directionality form-associated elements">auto-directionality form-associated
Copy link
Contributor

@fantasai fantasai Oct 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might consider calling this a "rendered-value input element" or something, since the distinction here is that we're rendering the contents of the value as visible content of the rendered element.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you consider Hidden inputs rendered? Maybe mostly-text-value? :-)

@zcorpan
Copy link
Member

zcorpan commented Oct 24, 2023

@nt1m the spec says to obscure input for password fields, so UAs could also obscure directionality (i.e. force LTR in the rendering of the bullets), right?

annevk added a commit to web-platform-tests/wpt that referenced this pull request Oct 24, 2023
For whatwg/html#9689.

Helps with #25569 as this area was barely tested.

(Also correct some typos while here.)
@annevk annevk merged commit 11114d0 into main Oct 24, 2023
2 checks passed
@annevk annevk deleted the annevk/direction-of-password-and-buttons branch October 24, 2023 13:29
webkit-commit-queue pushed a commit to annevk/WebKit that referenced this pull request Oct 24, 2023
https://bugs.webkit.org/show_bug.cgi?id=259440
rdar://113127508

Reviewed by Ryosuke Niwa.

This applies dir=auto to Hidden, Password, Submit Button, Reset Button,
and Button input types.

It also supports the dirname attribute for Password and Submit Button
input types and removes support for it from Number input types.

Tests are kept synchronized via
web-platform-tests/wpt#42427.

Standard is updated in whatwg/html#9689.

* LayoutTests/imported/w3c/web-platform-tests/css/selectors/dir-pseudo-on-input-element-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/selectors/dir-pseudo-on-input-element.html:
* LayoutTests/imported/w3c/web-platform-tests/html/dom/elements/global-attributes/dir-auto-form-associated.window-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/html/dom/elements/global-attributes/dir-auto-form-associated.window.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/html/dom/elements/global-attributes/dir-auto-form-associated.window.js: Added.
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/forms/attributes-common-to-form-controls/dirname-ltr-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/forms/attributes-common-to-form-controls/dirname-ltr.html:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/forms/attributes-common-to-form-controls/dirname-only-if-applies-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/forms/attributes-common-to-form-controls/dirname-only-if-applies.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/forms/attributes-common-to-form-controls/dirname-rtl-auto-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/forms/attributes-common-to-form-controls/dirname-rtl-auto.html:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/forms/attributes-common-to-form-controls/dirname-rtl-inherited-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/forms/attributes-common-to-form-controls/dirname-rtl-inherited.html:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/forms/attributes-common-to-form-controls/resources/dirname-iframe.html: Renamed from LayoutTests/imported/w3c/web-platform-tests/html/semantics/forms/attributes-common-to-form-controls/dirname-ltr-iframe.html.
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/forms/attributes-common-to-form-controls/resources/dirname.js: Added.
(onIframeLoadedDone):
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/forms/attributes-common-to-form-controls/resources/w3c-import.log: Added.
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/forms/attributes-common-to-form-controls/w3c-import.log:
* LayoutTests/platform/mac-wk1/imported/w3c/web-platform-tests/css/selectors/dir-pseudo-on-input-element-expected.txt:
* LayoutTests/platform/mac-wk1/imported/w3c/web-platform-tests/html/dom/elements/global-attributes/dir-auto-form-associated.window-expected.txt: Added.
* LayoutTests/platform/mac-wk1/imported/w3c/web-platform-tests/html/semantics/forms/attributes-common-to-form-controls/dirname-only-if-applies-expected.txt: Added.
* LayoutTests/platform/wpe/imported/w3c/web-platform-tests/css/selectors/dir-pseudo-on-input-element-expected.txt:
* LayoutTests/platform/wpe/imported/w3c/web-platform-tests/html/dom/elements/global-attributes/dir-auto-form-associated.window-expected.txt: Added.
* LayoutTests/platform/wpe/imported/w3c/web-platform-tests/html/semantics/forms/attributes-common-to-form-controls/dirname-only-if-applies-expected.txt: Added.
* Source/WebCore/html/BaseButtonInputType.cpp:
(WebCore::BaseButtonInputType::dirAutoUsesValue const):
* Source/WebCore/html/BaseButtonInputType.h:
* Source/WebCore/html/BaseTextInputType.h:
* Source/WebCore/html/FileInputType.cpp:
(WebCore::FileInputType::dirAutoUsesValue const):
* Source/WebCore/html/FileInputType.h:
* Source/WebCore/html/HiddenInputType.cpp:
(WebCore::HiddenInputType::appendFormData const):
(WebCore::HiddenInputType::dirAutoUsesValue const):
* Source/WebCore/html/HiddenInputType.h:
* Source/WebCore/html/ImageInputType.cpp:
(WebCore::ImageInputType::dirAutoUsesValue const):
* Source/WebCore/html/ImageInputType.h:
* Source/WebCore/html/PasswordInputType.cpp:
(WebCore::PasswordInputType::dirAutoUsesValue const): Deleted.
* Source/WebCore/html/PasswordInputType.h:
* Source/WebCore/html/SubmitInputType.cpp:
(WebCore::SubmitInputType::appendFormData const):
* Source/WebCore/html/TextFieldInputType.cpp:
(WebCore::TextFieldInputType::appendFormData const):

Canonical link: https://commits.webkit.org/269711@main
@dbaron
Copy link
Member

dbaron commented Oct 24, 2023

For what it's worth, the changes look reasonable to me as well. (Sorry for the delay!)

@aphillips
Copy link
Contributor

@zcorpan noted:

the spec says to obscure input for password fields, so UAs could also obscure directionality (i.e. force LTR in the rendering of the bullets), right?

They could (and probably should not exhibit input-side-changing behavior), but I wouldn't force LTR rendering of the bullets. If the dir of the control is RTL, it probably looks more natural (and it is what at least a couple of browsers do) to render the bullets according to that:

image

image

aarongable pushed a commit to chromium/chromium that referenced this pull request Oct 31, 2023
Following the spec change [1], we update the logic of what elements are
considered auto directionality form associated elements.

We also improve the logic for InputType::AppendToFormData to support the
dirname attribute for Password and Submit Button input elements.

[1] whatwg/html#9689

Change-Id: Ie5cd5abb4fcdb7d92aa9352a59b95a6496e9de28
Fixed: 1491346
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4977737
Commit-Queue: Di Zhang <[email protected]>
Reviewed-by: David Baron <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1217927}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Nov 7, 2023
… more form controls, a=testonly

Automatic update from web-platform-tests
HTML: make dir=auto and dirname apply to more form controls

For whatwg/html#9689.

Helps with web-platform-tests/wpt#25569 as this area was barely tested.

(Also correct some typos while here.)
--

wpt-commits: 364c325f6589ec43c8e0abd85dc3220c6592721d
wpt-pr: 42427
vinnydiehl pushed a commit to vinnydiehl/mozilla-unified that referenced this pull request Nov 7, 2023
… more form controls, a=testonly

Automatic update from web-platform-tests
HTML: make dir=auto and dirname apply to more form controls

For whatwg/html#9689.

Helps with web-platform-tests/wpt#25569 as this area was barely tested.

(Also correct some typos while here.)
--

wpt-commits: 364c325f6589ec43c8e0abd85dc3220c6592721d
wpt-pr: 42427
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Nov 8, 2023
… more form controls, a=testonly

Automatic update from web-platform-tests
HTML: make dir=auto and dirname apply to more form controls

For whatwg/html#9689.

Helps with web-platform-tests/wpt#25569 as this area was barely tested.

(Also correct some typos while here.)
--

wpt-commits: 364c325f6589ec43c8e0abd85dc3220c6592721d
wpt-pr: 42427

UltraBlame original commit: baa84c2ff0cb5d7c031e93e92f5694397affc2f5
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Nov 8, 2023
… more form controls, a=testonly

Automatic update from web-platform-tests
HTML: make dir=auto and dirname apply to more form controls

For whatwg/html#9689.

Helps with web-platform-tests/wpt#25569 as this area was barely tested.

(Also correct some typos while here.)
--

wpt-commits: 364c325f6589ec43c8e0abd85dc3220c6592721d
wpt-pr: 42427

UltraBlame original commit: baa84c2ff0cb5d7c031e93e92f5694397affc2f5
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Nov 8, 2023
… more form controls, a=testonly

Automatic update from web-platform-tests
HTML: make dir=auto and dirname apply to more form controls

For whatwg/html#9689.

Helps with web-platform-tests/wpt#25569 as this area was barely tested.

(Also correct some typos while here.)
--

wpt-commits: 364c325f6589ec43c8e0abd85dc3220c6592721d
wpt-pr: 42427

UltraBlame original commit: baa84c2ff0cb5d7c031e93e92f5694397affc2f5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
i18n-tracker Group bringing to attention of Internationalization, or tracked by i18n but not needing response. normative change topic: forms
7 participants