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
Similarly to #2034995: Test taxonomy term formatter RDFa output and other fields, we need to support RDFa for the link field formatter. The field formatter needs modification very similar to the taxonomy field formatter (see referenced issue).
Proposed resolution
Update the field formatter and add tests.
Remaining tasks
write patch.
User interface changes
none
API changes
none
Comment | File | Size | Author |
---|---|---|---|
#33 | interdiff.txt | 705 bytes | CharuAg |
#33 | 2188889-32.patch | 8.06 KB | CharuAg |
#30 | interdiff.txt | 3.24 KB | CharuAg |
#30 | 2188889-30.patch | 7.97 KB | CharuAg |
#28 | 2188889-28.patch | 7.69 KB | chrisfromredfin |
Comments
Comment #1
kay_v CreditAttribution: kay_v commentedso far just the change to rdf module to put attributes into the href rather than the parent container
Comment #2
ashepherd CreditAttribution: ashepherd commentedReviewed changes. Worked for both link and link_separate field formatters. Wrote test: LinkFieldRdfaTest.php, and it passed locally.
Comment #4
lokapujyaCombined the patches and fixed a whitespace error.
Comment #6
lokapujya4: 2188889-4-LinkFieldRdfaTest.patch queued for re-testing.
Comment #7
scor CreditAttribution: scor commentedGreat progress here, Jamie!!
There is a reference to the email field here, it should say "link field".
We should also test some cases where the link has a title (along with the url value), and various field formatter settings, such as 'Add rel="nofollow" to links' or 'Open link in new window'. These should not impact the RDFa, but we should test them anyways. It should be trivial to test with the help of #2203065: Adjust assertFormatterRdfa() parameters to allow for more advanced testing. Feel free to include that patch in this issue for now (commit it locally in your issue branch) while devising the test for the link formatter, once #2203065 is committed, we can remove and RTBC this link formatter issue.
Comment #8
chrisfromredfinThis incorporates the above comment feedback, as well as the feedback to test using different formatter settings. (In fact, that allowed us to discover that the URL as plain text version of the formatter was not displaying correct RDF, so that is also fixed with this patch).
NOTE: This patch incorporates as part of it the latest patch from #2203065 as well (#6 at the time of this writing), which I manually re-applied.
Comment #9
chrisfromredfinTrigger test bot. Still there is probably comment cleanup to do, etc.
Comment #11
chrisfromredfinOK, just looks like I'll need to pull in some of the parallel work being done with the date formatter.
Comment #12
chrisfromredfinThe tests failed because $item->_attributes isn't necessarily always set, so we need to check to make sure it's non-empty (i.e. we have a mapping defined) before merging in our attributes.
Comment #13
chrisfromredfin12: 2188889-12-LinkFieldRdfaTest.patch queued for re-testing.
Comment #15
chrisfromredfinRe-rolled, flinging to testbot.
Comment #17
chrisfromredfinWork in progress...
Comment #18
chrisfromredfinMore progress, and I know this fails the Link Formatter for internal links, but want to see why...
Comment #20
lokapujyareroll.
Comment #21
lokapujyaThis should get the tests working. Still needs improvement; Probably the expected value should be set up inside the testAllFormattersX() functions.
Why does assertFormatterRdfa() expect an absolute URL when we pass in a relative URL?
Comment #23
scor CreditAttribution: scor commentedThose are coding standard fixes, please create a follow up issue to fix those.
ditto.
The coding standards need to be verified throughout LinkFieldRdfaTest.php.
Given that those are not UI facing, let's spell the types 'external', 'internal' and 'front' all in lower case.
The RDFa will still be parsed and extract full URLs, even if those are relative, they will get expanded to match the localhost site.
good idea.
Comment #24
lokapujya1, 2. Ok, those can be done as part of #1533380: Make RDF module pass Coder Review
3. Adjusted some comments.
4. Done
Moved the expected value to the X() functions.
Comment #25
chrisfromredfinThe patch still applies clean.
Comment #26
scor CreditAttribution: scor commentedThe comment here should say something like: Piggyback on the metadata attributes which will be placed in the field template wrapper, and set the URL value in a content attribute.
let's keep the patch small (new line here).
Add a comment to say that we're generating a 'a' element here, which is a perfect fit for the RDFa metadata attribute.
rogue newline.
s/set/Set and missing period.
Comment #27
chrisfromredfinAddress all review notes in 26.
UPDATE: ignore this, it excludes an entire file.
Comment #28
chrisfromredfinAddress all review notes in 26.
Comment #29
scor CreditAttribution: scor commentedcomments should span to 80 chars
Comment #30
CharuAg CreditAttribution: CharuAg commentedAddress comment 29 and other documentation fixes.
Comment #31
scor CreditAttribution: scor commentedPatch looks good now thanks to the recent fixes by Charu. Good to go.
Comment #32
lokapujyaThe tests are trimming to different lengths. But the url is too short to make a difference.
Also, the assert message is the same for all the tests. If possible, we could change it for each assertion.
Comment #33
CharuAg CreditAttribution: CharuAg commentedAddressed comment 32
Comment #34
CharuAg CreditAttribution: CharuAg commentedComment #35
scor CreditAttribution: scor commentedURL is now more than 80 charracter long. back to RTBC.
Comment #36
webchickCommitted and pushed to 8.x. Thanks!
Comment #38
scor CreditAttribution: scor commentedand congrats @CharuAg for your first patch to core!
Comment #39
CharuAg CreditAttribution: CharuAg commentedThanks! :)