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

Fix the RegExp flag used for input pattern validation #3681

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 10 additions & 4 deletions lib/jsdom/living/nodes/HTMLInputElement-impl.js
Original file line number Diff line number Diff line change
Expand Up @@ -1025,10 +1025,16 @@ class HTMLInputElementImpl extends HTMLElementImpl {
// The pattern attribute should be matched against the entire value, not just any
// subset, so add ^ and $ anchors. But also check the validity of the regex itself
// first.
new RegExp(pattern, "u"); // eslint-disable-line no-new
regExp = new RegExp("^(?:" + pattern + ")$", "u");
} catch (e) {
return false;
regExp = new RegExp("^(?:" + pattern + ")$", "v");
} catch (_) {
// Exception may have been thrown because "v" was unsupported
// Fallback to "u" flag
try {
const pattern = this.getAttributeNS(null, "pattern");
regExp = new RegExp("^(?:" + pattern + ")$", "u");
} catch (e) {
return false;
}
}
if (this._hasAttributeAndApplies("multiple")) {
return !splitOnCommas(this._value).every(value => regExp.test(value));
Expand Down
4 changes: 2 additions & 2 deletions test/web-platform-tests/to-run.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1107,7 +1107,7 @@ formaction.html: [fail, Unknown]

DIR: html/semantics/forms/constraints

form-validation-validity-patternMismatch.html: [fail, Unknown]
Copy link
Member

Choose a reason for hiding this comment

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

Use fail-node18 here (and below) to signal the right support matrix, and the tests should pass, I think!

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for the tip! I added it, but there were more tests that fail. Essentially, any test where the input has a pattern will fail. Should I add more fail-node18 to those tests?

Copy link
Member

Choose a reason for hiding this comment

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

Oh, I see. Well, we don't want to make jsdom (or the pattern parts of it) completely unusable on Node 18, at least until it goes out of LTS support. So I guess we need a fallback, that try/catches with "v" and then falls back to "u".

form-validation-validity-patternMismatch.html: [fail-node18, v is in invalid flag for RegExp in Node 18]
form-validation-validity-valueMissing.html: [fail, 'Spec unclear (see https://github.com/whatwg/html/issues/5202)']
form-validation-willValidate.html:
"[INPUT in COLOR status] Must be barred from the constraint validation if it is readonly": [fail, Not implemented]
Expand Down Expand Up @@ -1190,7 +1190,7 @@ input-type-button.html: [fail, Depends on offsetWidth]
input-type-change-value.html: [fail, Unknown]
input-type-checkbox-switch.tentative.window.html: [fail, Unknown]
input-untrusted-key-event.html: [fail-slow, Not implemented]
pattern_attribute_v_flag.html: [fail, Unknown]
pattern_attribute_v_flag.html: [fail-node18, v is in invalid flag for RegExp in Node 18]
radio-disconnected-group-owner.html: [fail, Unknown]
range-2.html: [fail, step attribute not yet implemented]
range-restore-oninput-onchange-event.https.html: [fail-slow, Not implemented]
Expand Down