r91654 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r91653‎ | r91654 | r91655 >
Date:17:32, 7 July 2011
Author:sean_colombo
Status:reverted (Comments)
Tags:
Comment:
Put the OpenGraphMeta info into meta tags section of OutputPage. Makes it more portable (not all wikis do headItems the same).
Modified paths:
  • /trunk/extensions/OpenGraphMeta/OpenGraphMeta.php (modified) (history)

Diff [purge]

Index: trunk/extensions/OpenGraphMeta/OpenGraphMeta.php
——@@ -84,9 +84,9 @@
8585 $meta["fb:admins"] = $egFacebookAdmins;
8686
8787 foreach( $meta as $property => $value ) {
88 - if ( $value )
89 - //$out->addMeta($property, $value ); // FB wants property= instead of name= blech, is that even valid html?
90 - $out->addHeadItem("meta:property:$property", " ".Html::element( 'meta', array( 'property' => $property, 'content' => $value ) )."\n");
 88+ if ( $value ){
 89+ $out->addMeta("property:$property", $value );
 90+ }
9191 }
9292
9393 return true;

Follow-up revisions

RevisionCommit summaryAuthorDate
r93158Revert r91654 for now. This change alone breaks the code. It should be acompa...dantman02:57, 26 July 2011

Comments

#Comment by Dantman (talk | contribs)   02:51, 26 July 2011

This change breaks OpenGraphMeta on vanilla mediawiki. <meta property="og:title" content="..." /> becomes <meta name="property:og:title" content="..." /> and Facebook now completely ignores it. If there is a Wikia specific hack in place on Wikia's codebase making property: work, please commit it to MediaWiki's core trunk and include a way to feature test for it so that this extension can work outside of Wikia.

#Comment by SColombo~mediawikiwiki (talk | contribs)   03:10, 26 July 2011

Yikes! I didn't realize that was a Wikia-specific hack in OutputPage. I thought I checked, but now I see that it definitely isn't in core.

Change was here: http://trac.wikia-code.com/changeset/35596%20#file1

Will merge the change in and then try to make some test code. Is there an already-existing test that this would be a good addition to?

#Comment by Dantman (talk | contribs)   03:21, 26 July 2011

I think that code needs two tweaks before it's put into core:

  • We need something that can be feature tested. Like the MW_SUPPORTS_PARSERFIRSTCALLINIT constant was added for testing. property: is just part of the string and if we just drop that in there is no way to tell if the feature is supported or not, so the extension will break pre-1.19
  • property: is not "Open Graph" it's part of RDFa, should probably fix the comments before inclusion into core.

Status & tagging log