Problem/Motivation

David Rothstein pointed out in #1323830-57: Place title RDFa metadata inside entity HTML element that maybe the empty span element that we insert in the HTML to assert metadata in RDFa should be hidden explicitly in CSS.

Proposed resolution

One of two options:

1. Add the element-hidden (D7) / hidden (D8) class to the span element, like this:

      <span property="dc:title" content="title of something" class="rdf-meta hidden"></span>

2. Don't change the HTML markup, but instead add a CSS declaration for the rdf-meta class which we are already using in the span, which would live in a CSS file in the RDF module:

.rdf-meta {
  display: none;
}

Remaining tasks

decide & write patch

User interface changes

none

API changes

none

CommentFileSizeAuthor
#14 drupal7-rdf-mettag-hidden-2301955-14.patch883 byteser.pushpinderrana
#9 interdiff-2301955-9.txt802 byteslokapujya
#9 2301955-9.patch947 byteslokapujya
#6 drupal8-rdf-mettag-hidden-2301955-6.patch712 byteser.pushpinderrana
#6 interdiff-2301955-2-6.txt842 byteser.pushpinderrana
#2 drupal8-rdf-mettag-hidden-2301955-2.patch802 byteser.pushpinderrana
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

scor’s picture

scor CreditAttribution: scor commented

I personally prefer option #2.

er.pushpinderrana’s picture

er.pushpinderrana CreditAttribution: er.pushpinderrana commented
Status: Active » Needs review
FileSize
drupal8-rdf-mettag-hidden-2301955-2.patch802 bytes

@scor, Please review attach patch, followed first approach as instead of writing new css, prefer to go with existing css class.

Please confirm if required, I would go with #2 approach.

Thanks!

scor’s picture

scor CreditAttribution: scor commented
Status: Needs review » Needs work

Even if we went with option #1, this patch should set the class using an array, not a string of classes.

I'd like to hear what people think of option #2 vs option #1.

Zarabadoo’s picture

Zarabadoo CreditAttribution: Zarabadoo commented

I would personally opt for #1 and use the hidden class. Yes, it adds a few bytes per instance to the page, but it is going to have minimal effect. These core Drupal classes have meaning and are established patterns in the project. There is no sense in reinventing the wheel because we save a miniscule amount on page size. I think consistency is more important.

scor’s picture

scor CreditAttribution: scor commented

thanks for chiming in Al. Let's wait for @er.pushpinderrana to update the patch so we can move forward with this.

er.pushpinderrana’s picture

er.pushpinderrana CreditAttribution: er.pushpinderrana commented
Status: Needs work » Needs review
FileSize
interdiff-2301955-2-6.txt842 bytes
drupal8-rdf-mettag-hidden-2301955-6.patch712 bytes

@scor and @Zarabadoo, thankyou for your valuable feedback. Please review attached patch.

scor’s picture

scor CreditAttribution: scor commented
Status: Needs review » Reviewed & tested by the community

Manually verified that we're now outputting class="rdf-meta hidden". Good!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Needs work 
    // Add a class so that developers viewing the HTML source can see why there
    // are empty <span> tags in the document. The class can also be used to set
    // a CSS display:none rule in a theme where empty spans affect display.

This comment just above in template_preprocess_rdf_metadata seems obsolete - no?

lokapujya’s picture

lokapujya
Primary language English
Location Boston
CreditAttribution: lokapujya commented
Status: Needs work » Needs review
FileSize
2301955-9.patch947 bytes
interdiff-2301955-9.txt802 bytes

Fixed the comment as suggested above.

scor’s picture

scor CreditAttribution: scor commented
Status: Needs review » Reviewed & tested by the community

Good point, Alex. Thanks for fixing the docs, Jamie. Sending back to the RTBC queue.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Fixed

Committed 675f72f and pushed to 8.x. Thanks!

  • alexpott committed 675f72f on 8.x 
    Issue #2301955 by er.pushpinderrana, lokapujya | scor: Ensure RDFa...
scor’s picture

scor CreditAttribution: scor commented
Version: 8.x-dev » 7.x-dev
Status: Fixed » Patch (to be ported)
Issue tags: +Novice

yay! now onto D7.

er.pushpinderrana’s picture

er.pushpinderrana CreditAttribution: er.pushpinderrana commented
Status: Patch (to be ported) » Needs review
FileSize
drupal7-rdf-mettag-hidden-2301955-14.patch883 bytes

D7 patch. Please review.

scor’s picture

scor CreditAttribution: scor commented
Status: Needs review » Reviewed & tested by the community

@er.pushpinderrana you are very fast! thanks!

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 14: drupal7-rdf-mettag-hidden-2301955-14.patch, failed testing.

Status: Needs work » Needs review

er.pushpinderrana queued 14: drupal7-rdf-mettag-hidden-2301955-14.patch for re-testing.

scor’s picture

scor CreditAttribution: scor commented
Status: Needs review » Reviewed & tested by the community

flaky testbot.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 14: drupal7-rdf-mettag-hidden-2301955-14.patch, failed testing.

dcam’s picture

dcam CreditAttribution: dcam commented
Status: Needs work » Reviewed & tested by the community

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 14: drupal7-rdf-mettag-hidden-2301955-14.patch, failed testing.

lokapujya’s picture

lokapujya
Primary language English
Location Boston
CreditAttribution: lokapujya commented
Status: Needs work » Reviewed & tested by the community
mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford commented
Related issues: +#1323830: Place title RDFa metadata inside entity HTML element

Great that this is RTBC. Adding related issue #1323830: Place title RDFa metadata inside entity HTML element

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 14: drupal7-rdf-mettag-hidden-2301955-14.patch, failed testing.

lokapujya’s picture

lokapujya
Primary language English
Location Boston
CreditAttribution: lokapujya commented
Status: Needs work » Reviewed & tested by the community

dcam queued 14: drupal7-rdf-mettag-hidden-2301955-14.patch for re-testing.

David_Rothstein’s picture

David_Rothstein CreditAttribution: David_Rothstein commented
Status: Reviewed & tested by the community » Fixed
Issue tags: +7.33 release notes

Committed to 7.x - thanks!

  • David_Rothstein committed fa2e36b on 7.x 
    Issue #2301955 by er.pushpinderrana, lokapujya | scor: Ensure RDFa...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.