The 2026 Drupal Business Survey is open. Your response will help shape the future of the Drupal business ecosystem.Problem/Motivation
There are some documentation links that have invalid HTML. According the HTML specification, every attribute in a HTML tag have a pair (attribute, value) with a notation like (with double quotes). In the documentation, there are some links that have the value of the @href attribute with only one quote.
Steps to reproduce
Proposed resolution
Fix the quoting of the href attribute.
Use this to find instances.
$ git grep -ri "/code>
Remaining tasks
Review
Commit
User interface changes
API changes
Data model changes
Release notes snippet
I attach a patch that solves this little bug.
| Comment | File | Size | Author |
|---|
Issue fork drupal-898656
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
- 898656-some-documentation-links
changes, plain diff MR !8130
Comments
Comment #1
jacineHi @rvilar, thank you for the patch! You are correct and this should be fixed.
However, the following lines don't have any links or invalid HTML, so they would need to removed from this patch. Also, I'm 99% positive that it's too late to get this fixed in Drupal 7, even though it would be nice, because I believe it would require changes to translations that have already been made, and we are well past string freeze. So, I'm going to mark this 8.x unless someone can prove me wrong.
Powered by Dreditor.
Comment #2
webchickWe can still break strings at this phase for bug fixes, but it is my understanding that single-quoted attributes, though slightly odd, are actually fine from a W3C compliance POV. Is there some documentation out there to the contrary?
Comment #3
jacineThanks @webchick ;)
Single quotes validate, so it's not technically a W3C compliance issue. It's more of a consistency/code style issue. We need to pick one, which we already have, and stick with it, so this should be fixed. Whether that happens in 7 or 8, is up to you.
Comment #4
rvilarThanks @Jacine and @webchick
In XML specification the double quotes is a recommendation and not, how I thought, an error. Excuse me for this little issue.
I'm think, like @Jacine, that now it's more a code style issue than an error, like I expected. i think that it has to be fixed for consistency.
Comment #5
jacineThe patch still needs work, per my comment in #1. Since it's not technically a bug, and there is no patch that's ready, I'm fine pushing this to 8.x.
Comment #6
dave reidI would also rather use the format of
t("<a href=\"value\">")rather than having to change all strings and possibly cause lots of unnecessary re-rolls.Comment #7
albert volkman commentedBringing this issue back from the grave :)
Comment #10
albert volkman commentedRe-roll
Comment #11
jhedstromComment #12
ravi.khetri commentedRerolled
Comment #14
jyotisankar commentedComment #16
ravi.khetri commentedComment #17
jyotisankar commentedComment #19
jain_deepak commentedRerolled
Comment #21
albert volkman commentedIncorrect spacing here.
Comment #22
manningpete commentedLast patch applies.
Comment #23
sumit.prajapati commentedIssue in numbetr #21 has been addressed please see the patch.
Comment #24
jhedstromThis issue is specific to using double quotes in link tags. It shouldn't replace the use of double quotes where they avoid needing to escape single quotes, since that is intentional according to the coding standards.
Comment #25
goldlilys commentedShouldn't this be in Needs Review rather than Needs Work since the last submitted patch is passing?
Comment #26
jhedstromIt's at needs work based on the feedback in #24.
Comment #27
deepakaryan1988Removing sprint weekend tag!! and adding documentation tag
As suggested by @YesCT
Comment #28
deepakaryan1988Sorry, these issues were actually worked on during the 2015 Global Sprint
Weekend https://groups.drupal.org/node/447258
Comment #29
virajthakrar commentedChanges updated as per comment #24 and update code as per drupal coding standards.
Comment #31
virajthakrar commentedUpdated according to coding standards and Comment #24.
Comment #41
quietone commentedIt has been some time since this has been worked on.
I used this to find instances,
grep -ri "<a href='" core| awk -F: '{print $1}' | sort | nl, which I am sure could be improved.With this patch the following are found:
The only file of interest there is core/modules/views/tests/src/Functional/GlossaryTest.php. The lines there are
which I don't think need to be changed.
Comment #45
smustgrave commentedThis issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request as a guide.
At this time we will need a D10 version of patch #41 and a recheck using the same grep as before.
Comment #46
Abhisheksingh27 commentedAdding reroll for 10.1.x as the patch #41 failed to apply in drupal 10.
please review
Comment #47
Abhisheksingh27 commentedAdding #46 patch again with improved coding satandards.
Comment #48
smustgrave commented@Abhisheksingh27 please include interdiffs with patches.
Following what @quietone ran
grep -ri "<a href='" core| awk -F: '{print $1}' | sort | nland gotSo mainly just node_modules stuff. Did see GlossaryTest as mentioned but as quietone pointed out this should be fine.
Updating remaining tasks in issue summary.
Comment #49
xjmThanks @quietone and @smustgrave.
Re:
GlossaryTest, is the comment maybe just wrong? If it's true, then the whole view has bad markup. If it's false, it should be changed. Manually testing now...Comment #50
xjmI confirmed
GlossaryTestjust has an incorrect comment -- the attribute is correctly double-quoted in the actual view -- so we might as well fix that comment too, since it's part of the same pattern.Scope creep here.
These are changing the URLs themselves as well as how they are quoted. And, in fact, they seem to be reverting another fix. The URL in HEAD (without the
/8/) appears to be the correct one, and using the/8/version triggers a redirect.NW mainly for point 3; the others are trivial. Thanks!
Comment #53
ksenzeeI worked with my team (@dcmorris, @jmarcella, @tinycanary) at Drupalcon Portland 2024 to resurrect this patch and see what's left of it in 11.x. We'll post results soon.
Comment #56
gauravvvv commentedComment #57
needs-review-queue-bot commentedThe 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.)
Comment #58
quietone commentedLet's finish this small change.
From #50
1. Fixed
2. No book module on 11.x
3. Fixed.
I tested the changes to Umami by installing and confirming the output on the about page for both languages.
And a final check with this grep shows that there are no other href attributes starting with a single quote.
Comment #59
smustgrave commented2 small comments on MR.
Comment #60
quietone commentedComment #61
smustgrave commentedThanks @quietone my 2 questions are answered.
Comment #64
nod_Committed b85397f and pushed to 11.x. Thanks!
Comment #66
xjmCrediting myself for my committer reviews which should for sure have been credited under our policy. :)