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

LibWeb/HTML: Make disabled items immutable & fix disabled activations #3120

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Jaycadox
Copy link

@Jaycadox Jaycadox commented Jan 3, 2025

Follows HTMLInputElement::activation_behavior closer to specifications.

Previously, these disabled form input elements wouldn't be able to be activated via user interaction, but they would be able to be activated via JavaScript (at least in some conditions).

As for the changes in FormAssociatedTextControlElement::is_mutable, it seems to me that it makes sense for disabled input items to also not be mutable, though I didn't personally find the spec entirely clear in that regard. Afterwards, as a sanity check, I looked at what Firefox and WebKit do, and they seem to also register a form input element as immutable if it is disabled, I believe for all form input types, thus I believe this change is correct.

Fixes 3 WPT tests.

@@ -173,7 +173,7 @@ class FormAssociatedTextControlElement : public FormAssociatedElement
bool has_scheduled_selectionchange_event() const { return m_has_scheduled_selectionchange_event; }
void set_scheduled_selectionchange_event(bool value) { m_has_scheduled_selectionchange_event = value; }

bool is_mutable() const { return m_is_mutable; }
bool is_mutable() const { return m_is_mutable && enabled(); }
Copy link
Contributor

Choose a reason for hiding this comment

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

consider placing some ad-hoc or fixme annotation. not sure whether it is the best approach, the spec says

A form control that is disabled must prevent any click events that are queued on the user interaction task source from being dispatched on the element.

which might be the correct approach (implemented in other place).

not a maintainer though, wait for input from their side for proper guidance.

Copy link
Author

@Jaycadox Jaycadox Jan 4, 2025

Choose a reason for hiding this comment

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

I'm open to the idea of changing it. But to me, it makes sense that disabled form inputs are also not mutable. I might be mistaken here, but I think the only properties that are actually exposed are "disabled" and "readonly", and that both being disabled and readonly makes an element not mutable, but being disabled entails other behaviour on top of that.

So perhaps it would make more sense to have an "is_disabled/enabled" and an "is_readonly" boolean which are seperate, and both used to calculate an "is_mutable" function.

Input events performed by user actions would work as intended on
disabled input elements, but they could still be activated by JS, to
fix this, the input activation behavior specification was followed
more closely.

As well, the check for mutability of an input element now requires the
 element to be enabled. I believe that this is the intent of the
 specifications and other browsers follow in this regard, as well, I
 do not see how the specifications would result in the correct
 behavior if disabled input elements could also be mutable.
@Jaycadox Jaycadox force-pushed the proper-input-element-mutability branch from 0155635 to ba51ef7 Compare January 4, 2025 19:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants