Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
There are a number of instances where we link to the PHP manual for more information on date/time formats. But the PHP docs have changed and the current link (https://www.php.net/manual/function.date.php) documents the date() function rather than the parameters that are useful for creating date formats, which is now available at https://www.php.net/manual/datetime.format.php#refsect1-datetime.format-...
One example of this is the help text for the date/time format at /admin/config/regional/date-time/formats/manage/[format]
Screenshot of this text:
Proposed resolution
Update the link in the help text to https://www.php.net/manual/en/datetime.format.php#refsect1-datetime.form...
Remaining tasks
Patch- Review
User interface changes
N/A
API changes
N/A
Data model changes
N/A
Release notes snippet
N/A
| Comment | File | Size | Author |
|---|---|---|---|
| #31 | 3172582-29.patch | 6.73 KB | g-brodiei |
| #26 | interdiff_24-26.txt | 2.9 KB | sarvjeetsingh |
| #26 | 3172582-26.patch | 7.37 KB | sarvjeetsingh |
| #24 | interdiff-19_24.txt | 6.68 KB | g-brodiei |
| #24 | 3172582-24.patch | 7.32 KB | g-brodiei |
Comments
Comment #2
pameeela CreditAttribution: pameeela commentedPatch for this change attached.
Comment #3
pameeela CreditAttribution: pameeela commentedComment #4
larowlanLooks good to me.
The link used to be correct (I've had it bookmarked for years and only recently updated my bookmark), so I think this is more a case of the manual being updated at the PHP end.
Comment #5
pameeela CreditAttribution: pameeela commentedUpdated IS since @larowlan confirms that the php docs were updated, so this is actually a fix rather than a task!
Comment #6
mstrelan CreditAttribution: mstrelan commentedIt's probably best to leave the language prefix out of the URL.
Interestingly other translations still use the date() page to document the formats.
Comment #7
mstrelan CreditAttribution: mstrelan commentedAlso the patch is missing
href=".Comment #8
pameeela CreditAttribution: pameeela commentedWhoops, fixed!
Very strange, some languages have it both places, e.g.:
https://www.php.net/manual/de/datetime.format.php#refsect1-datetime.form...
https://www.php.net/manual/de/function.date.php#refsect1-function.date-p...
Comment #9
pameeela CreditAttribution: pameeela commentedComment #10
quietone CreditAttribution: quietone as a volunteer commentedI applied that patch and do not see the change at /admin/config/regional/date-time/formats/manage/medium?destination=/admin/config/regional/date-time.
I then did
grep -ri function.date.php core/moduleswhich returned 6 results. Maybe all of these instances should be changed?Comment #11
pameeela CreditAttribution: pameeela commentedYikes, I really punted this one! Thanks @quietone for pointing this out. I reviewed the other instances where this link is used and updated them, because I do think they all intend to point to the parameters rather than the function docs.
Just one note, I left the anchor off of the link in
FieldDateTest.phpto keep the comment under 80 lines. With the anchor the link itself is longer than 80 chars and I was not sure how to handle this, so decided to dodge it :) This use is only in a test so I think it's fine to link to the page.Comment #12
pameeela CreditAttribution: pameeela commentedOne final change, added
target="_blank"to the instances that didn't already have it, since this is in the UI so presumably you would like to keep your place. Also added a screenshot of the change in action:Comment #13
larowlanthanks @mstrelan and @quietone - I really dropped the ball on my review here 😂
Comment #14
larowlanComment #15
g-brodieiApplied #12 patch to 9.1.x , all occurence of string "function.date.php" related links have been successfully replaced by patch in with updated link https://www.php.net/manual/datetime.format.php#refsect1-datetime.format-....
+1 on skipping the anchor in FieldDateTest.php, setting to RTBC.
Comment #17
quietone CreditAttribution: quietone as a volunteer commentedClosed #3173408: Outdated link to PHP date format documentation as a duplicate, adding credit for the work on the patch over there.
Comment #18
alexpottRe #12 and adding target="_blank" everywhere - this has come up in #2652272: Target="_blank" attribute add to external links. - and there was pushback from @yoroy and @catch about adding this everywhere. I think we should only fix the links here and not add it everywhere.
Comment #19
pameeela CreditAttribution: pameeela commentedThanks @alexpott, I should have searched for a policy on this.
The attached patch removes it from all of the links. I read through that issue and #2702881: [policy, no patch] Formalize how external links are handled in core and it seems the consensus is not to use it, so we should remove the existing instances of it while we are here I think? That way it is at least consistent and it's easier for the user to know what to expect.
I marked this for no tests given the nature of this change.
Comment #20
mstrelan CreditAttribution: mstrelan commentedWhile we're at it, should we (in another issue) replace all occurrences of
http://php.net/withhttps://www.php.net/? And should the URLs be passed as replacement arguments rather than being part of the translatable string? There are examples of both in core.Comment #21
quietone CreditAttribution: quietone as a volunteer commented@mstrelan, good idie. I did some searching and there are other instances of http being used for external links. I mean not just php.net.
Setting to NW for the follow up to be made.
Comment #22
pameeela CreditAttribution: pameeela commentedI'm confused, this is NW until another ticket is created for an unrelated task? I have to say I don't really see the point of updating when the links work fine as is?
Comment #23
quietone CreditAttribution: quietone as a volunteer commentedWell, similar. Anyway, back to NR
Also todo. #20.
Comment #24
g-brodieiIf updating link about replacing all occurrence of http://php.net with https://www.php.net within core is what's postponing this issue to move further aside from the external link policy (which is yet to be decided, inactive for 4 month now). #2702881: [policy, no patch] Formalize how external links are handled in core
Personally I'd like to support @pameeela to push this issue forward with a slight change of patch.
That we simply update the link without making changes to the domain name (e.g. http://php.net to https;//www.php.net). That way at least it's updated in the meantime and it really benefits all Drupal users who has trouble finding the correct resource to configure DateTime fields.
I see that currently php.net can automatically redirect users from http://php.net to https://www.php.net, so like @mstrelan had said, we can do that in bulk in another issue, and await the decision of external link policy, but not be the reason from us updating the links.
Add tiny variance to #19 patch.
Comment #25
alexpott@g-brodiei I don't think http vs https is holding up this issue. I think this issue should be scope to only fix the link. It should not add or remove a
target="_blank"from any of the links it is fixing as that is out of scope. Given that php.net automatically forwards people to https://www.php.net/manual/en/datetime.format.php#refsect1-datetime.form... I think that that should be the link we use.The patch here can be made with a find and replace.
Comment #26
sarvjeetsingh CreditAttribution: sarvjeetsingh as a volunteer and at QED42 for Drupal India Association commentedAs per #25 i have added
target="_blank"to all the places from where it was removed.Comment #27
g-brodieiHahaha, thanks for pointing that out @alexpott.
Glad we pushed a bit forward on this issue.
Comment #28
g-brodieiLet me straighten this out due to my lack of understanding for English, got my head clearer in the morning.
We should update the link to the final redirect destination related to function.date.php, which is https://www.php.net/manual/en/datetime.format.php#refsect1-datetime.form....
Adding and Removing target="_blank" is out of scope. Awaiting #2702881: [policy, no patch] Formalize how external links are handled in core.
The updated link from http://php.net => https://www.php.net should not include other links that is out of scope. (e.g. other php.net related links within core), the alignment of this work should be done in another issue.
Comment #29
mstrelan CreditAttribution: mstrelan commentedThat is true if php.net thinks you want the English version. See #6, I think /en/ should be left out.
Comment #30
alexpott#29 is spot on. We should leave out the en - and let php.net direct you to the most appropriate language.
Comment #31
g-brodieiProviding patch base points written #28.
7 out of 8 links in 9.1.x core related to function.date.php is updated to support user experience in use of date/time format.
The link within FieldDateTest.php is not altered as it appears in comment, thus should be left out.
This updated link doesn't have /en/ langcode within so it redirects to browser's language preference.
Comment #32
g-brodieiLoL, ohhh no, I pressed too quick, the patch number is wrong, but the content is correct!
Thanks for pointing that out. yeah I left out the /en/ part. ;)
Comment #33
cobenash#31 Confirmed.
Comment #34
alexpottCommitted and pushed ad3f34c127 to 9.1.x. Thanks.
As this involves UI strings that have been translated we can only fix this in 9.1.x