r74203 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r74202‎ | r74203 | r74204 >
Date:14:12, 3 October 2010
Author:dantman
Status:resolved (Comments)
Tags:
Comment:
Add to OutputPage::headElement a OutputPageBodyAttrs hook for extensions and a bodyAttrs callback to Skin so that Extensions and Skins are able to add attributes and classes to the <body> tag without neeing to do insane things.
Modified paths:
  • /trunk/phase3/docs/hooks.txt (modified) (history)
  • /trunk/phase3/includes/OutputPage.php (modified) (history)
  • /trunk/phase3/includes/Skin.php (modified) (history)

Diff [purge]

Index: trunk/phase3/docs/hooks.txt
@@ -1193,6 +1193,14 @@
11941194 $parserOutput: the parserOutput (object) that corresponds to the page
11951195 $text: the text that will be displayed, in HTML (string)
11961196
 1197+'OutputPageBodyAttrs': called when OutputPage::headElement is creating the body
 1198+tag to allow for extensions to add attributes to the body of the page they might
 1199+need. Or to allow building extensions to add body classes that aren't of high
 1200+enough demand to be included in core.
 1201+$out: The OutputPage which called the hook, can be used to get the real title
 1202+$sk: The Skin that called OutputPage::headElement
 1203+&$bodyAttrs: An array of attributes for the body tag passed to Html::openElement
 1204+
11971205 'OutputPageCheckLastModified': when checking if the page has been modified
11981206 since the last visit
11991207 &$modifiedTimes: array of timestamps.
Index: trunk/phase3/includes/Skin.php
@@ -568,6 +568,15 @@
569569
570570 return "$numeric $type $name";
571571 }
 572+
 573+ /**
 574+ * This will be called by OutputPage::headElement when it is creating the
 575+ * <body> tag, skins can override it if they have a need to add in any
 576+ * body attributes or classes of their own.
 577+ */
 578+ function bodyAttributes( $out, &$bodyAttrs ) {
 579+ // does nothing by default
 580+ }
572581
573582 /**
574583 * URL to the logo
Index: trunk/phase3/includes/OutputPage.php
@@ -2276,6 +2276,9 @@
22772277 $bodyAttrs['class'] .= ' ' . Sanitizer::escapeClass( 'page-' . $this->getTitle()->getPrefixedText() );
22782278 $bodyAttrs['class'] .= ' skin-' . Sanitizer::escapeClass( $wgUser->getSkin()->getSkinName() );
22792279
 2280+ $sk->bodyAttributes( $this, $bodyAttrs ); // Allow skins to add body attributes they need
 2281+ wfRunHooks( 'OutputPageBodyAttrs', array( $this, $sk, &$bodyAttrs ) );
 2282+
22802283 $ret .= Html::openElement( 'body', $bodyAttrs ) . "\n";
22812284
22822285 return $ret;

Comments

#Comment by Brion VIBBER (talk | contribs)   22:45, 3 October 2010

My inclination would be to have Skin::bodyAttributes() return an array rather than using an out-param, to be more consistent with the rest of the class's interface, then merge in... if it would be preferable to keep them, then maybe call it Skin::addToBodyAttributes() by analogy with Skin::addToSidebar()?

Hook should probably be 'OutputPageBodyAttributes' as well, rather than having both 'Attrs' and 'Attributes' forms floating around.

#Comment by Siebrand (talk | contribs)   15:45, 23 October 2010

Dantman, please address this issue. Thanks.

#Comment by Dantman (talk | contribs)   18:16, 23 October 2010

I already committed r74245 to fix, didn't get associated?

#Comment by 😂 (talk | contribs)   18:21, 23 October 2010

It doesn't know it's related unless you mention "r74245" in the commit summary.

#Comment by Siebrand (talk | contribs)   18:27, 23 October 2010

Marking fixed because brion marked the follow-up as OK.

Status & tagging log