Problem/Motivation
This is a follow up of #2828556: Display meaningful error messages according to the link type
The default HTML5 validation error for url
input type is set by the browser, and most of them just display a message that does not really help the user to figure out how to correct an invalid link such as example.com
, which is missing the protocol. In Firefox, the default validation error message is Please enter a URL.
Note: this validation is done only for LinkItemInterface::LINK_EXTERNAL
, external links only.
Proposed resolution
Add a custom validation error message, such as Enter a full URL, such as http://example.com
Remaining tasks
Send patch
User interface changes
Changes the HTML5 validation error message for external links.
API changes
none
Data model changes
none
Screenshots
Default validation error message:
Custom validation error message:
Comment | File | Size | Author |
---|---|---|---|
#27 | chrome_after_patch.png.jpg | 20.99 KB | gaurav-mathur |
#27 | chrome_before_patch.png.jpg | 22.64 KB | gaurav-mathur |
#27 | after-patch.jpg | 22.99 KB | gaurav-mathur |
#27 | before_patch.png.jpg | 25.65 KB | gaurav-mathur |
#25 | interdiff-2861316-24_25.txt | 1.26 KB | Anchal_gupta |
Comments
Comment #2
cilefen CreditAttribution: cilefen commentedComment #3
cilefen CreditAttribution: cilefen commentedWe need the new array syntax and I assume, translation. The message needs a period because it's a sentence.
Comment #4
Mac_Weber CreditAttribution: Mac_Weber as a volunteer commented@cilefen thank you for the fast review.
Comment #6
Mac_Weber CreditAttribution: Mac_Weber as a volunteer commentedNew screenshot of the custom message.
Comment #7
wturrell CreditAttribution: wturrell as a volunteer commentedComment #15
ranjith_kumar_k_u CreditAttribution: ranjith_kumar_k_u at Zyxware Technologies commentedThe last patch applied successfully on 9.2 and it works fine
Before patch
After patch
Comment #19
rootworkKind of incredibly to me, this patch still applies in 9.5.x.
Before:
After:
This is essentially just a string change (well, addition). I'm going to mark it as RTBC because I think it's uncontroversial.
I do wonder if it'd be worth giving the example "https" instead of "http" now in 2022, but we'd need to change the field help text as well, and this patch still fixes what is otherwise a confusing error message, so that could be a follow-up.
Updating issue summary with new screenshots.
Comment #20
quietone CreditAttribution: quietone at PreviousNext commentedThe patch is being tested against an outdated version of Drupal. I am re-uploading the patch so it will test with 9.5.x
Comment #21
Abhijith S CreditAttribution: Abhijith S as a volunteer and at Zyxware Technologies commentedApplied patch #20 on 9.5.x and it working fine.
Attaching screenshot for reference.
After Patch;
Comment #22
rootworkComment #23
quietone CreditAttribution: quietone at PreviousNext commented@Abhijith S, the patch in #20 is the same as in #4, updated screenshots were added in #19. There was not need to do the work to make another set of screenshot, they are adding noise to the issue. Therefor, credit has been removed per How is credit granted for Drupal core issues.
Comment #24
alexpott$this->t()
This is super tricky. Using a translated string here means we're going to translate the string right away - but also we need to think about what a translator might put into the string and if it will break it. For example what happens if the translation contains a single '.
Plus there is no test coverage.
In the issue summary it says this is only added for links that support external links but that does not appear to be true.
Comment #25
Anchal_gupta CreditAttribution: Anchal_gupta at Srijan | A Material+ Company for Drupal India Association commentedI have attached the patch and addressed the #24
Please review it
Comment #26
alexpott#24.2 and #24.3 are not addressed.
Comment #27
gaurav-mathur CreditAttribution: gaurav-mathur at Dotsquares Ltd. commentedPatch #20 tested and applied successfully on Drupal 9.5.x. This patch works when url field is empty. Images attached for reference
Comment #30
SKAUGHTre: needs work
- overall, summary should be updated to include both internal and external. This phase should be tied to the field instance in use. #2828556: Display meaningful error messages according to the link type is meant to have those messages. [see: link-error_messages_by_link_type-2828556-30.patch ]
- js in patch is using direct
oninvalid
attribute.scope too narrow, see below.
A behavior for Drupal js seem like a better way to get this connected thru all form items. certainly Drupal.t() and plural can be handled there.
The scope is looking at Link, but is really this is where this new library should be able to continue to connect around form items that have 'required_error' to have a data attribute added into the form item render and use those strings for this level of (pre-submit) javascipt messaging to work more ideally.
Comment #31
SKAUGHT