- Status changed to Needs review
about 1 year ago 10:19pm 29 July 2024 - Status changed to Needs work
about 1 year ago 10:24pm 29 July 2024 The Needs Review Queue Bot → tested this issue.
While you are making the above changes, we recommend that you convert this patch to a merge request → . Merge requests are preferred over patches. Be sure to hide the old patch files as well. (Converting an issue to a merge request without other contributions to the issue will not receive credit.)
- 🇺🇸United States dcam
I closed 🐛 Link field doesn't sufficiently validate input Needs work as a duplicate. I updated the issue summary with info uncovered by that issue.
- First commit to issue fork.
- Merge request !13036Issue #2652236 by quietone, vakulrai, pguillard, logickal, ghost of drupal... → (Open) created by riyas_nr
In the previous patch the
URL
constraints failed because the value was passed directly to the parent validator.Updates in this patch:
- Created a Symfony Url constraint object and passed it to the parent validate().
- Added allowed protocols and used the child constraint message.
- Applied the constraint only for external URLs.
- Added test coverage for invalid characters, multiple comma-separated URLs, and other edge cases.
This ensures external links are properly validated and the behavior is covered by tests.
- 🇺🇸United States dcam
@riyas_nr I haven't done an in-depth review of the MR yet, but I notice we're adding a condition for
mailto:
URLs. There's no test coverage for it though. Is it possible to add a couple of test cases toLinkItemUrlValidationTest::getTestLinks()
for them?