r113223 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r113222‎ | r113223 | r113224 >
Date:10:13, 7 March 2012
Author:nikerabbit
Status:ok (Comments)
Tags:
Comment:
My proposed fix to bug 34987: gender not working in many special pages.
I haven't checked if there are other places whereh context is set to Message class, but if there are they might need a fix too.
Modified paths:
  • /trunk/phase3/includes/Message.php (modified) (history)
  • /trunk/phase3/includes/SpecialPage.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/SpecialPage.php
@@ -768,7 +768,15 @@
769769 // Works fine as the first parameter, which appears elsewhere in the
770770 // code base. Sighhhh.
771771 $args = func_get_args();
772 - return call_user_func_array( array( $this->getContext(), 'msg' ), $args );
 772+ $message = call_user_func_array( array( $this->getContext(), 'msg' ), $args );
 773+ // RequestContext passes context to wfMessage, and the language is set from
 774+ // the context, but setting the language for Message class removes the
 775+ // interface message status, which breaks for example usernameless gender
 776+ // invokations. Restore the flag when not including special page in content.
 777+ if ( !$this->including() ) {
 778+ $message->setInterfaceMessageFlag( true );
 779+ }
 780+ return $message;
773781 }
774782
775783 /**
Index: trunk/phase3/includes/Message.php
@@ -90,8 +90,7 @@
9191 * ->plain();
9292 * @endcode
9393 *
94 - * @note You cannot parse the text except in the content or interface
95 - * @note languages
 94+ * @note You can parse the text only in the content or interface languages
9695 *
9796 * @section message_compare_old Comparison with old wfMsg* functions:
9897 *
@@ -342,6 +341,18 @@
343342 }
344343
345344 /**
 345+ * Allows manipulating the interface message flag directly.
 346+ * Can be used to restore the flag after setting a language.
 347+ * @param $value bool
 348+ * @return Message: $this
 349+ * @since 1.20
 350+ */
 351+ public function setInterfaceMessageFlag( $value ) {
 352+ $this->interface = (bool) $value;
 353+ return $this;
 354+ }
 355+
 356+ /**
346357 * Enable or disable database use.
347358 * @param $value Boolean
348359 * @return Message: $this

Follow-up revisions

RevisionCommit summaryAuthorDate
r113745Follow-up r113223: set the "interface" flag in Message to true by default and...ialex18:41, 13 March 2012

Comments

#Comment by MaxSem (talk | contribs)   11:26, 7 March 2012

Looks a bit contradictory to other chained-call function names which don't start with set.

#Comment by Nikerabbit (talk | contribs)   11:38, 7 March 2012

I had makeInterfaceMessage, isInterfaceMessage (look like getter) and interfaceMessage and some others in my mind. At least this should be clear, although I'm open to suggestions.

#Comment by IAlex (talk | contribs)   11:37, 7 March 2012

Message::setInterfaceMessageFlag() is good, but I would rather call it with true from Message::setContext() so that it works in all IContextSource::msg() calls and set it back to false when the special page is included.

#Comment by Nikerabbit (talk | contribs)   11:40, 7 March 2012

It makes sense for the context provider to define the correct value, since it should know what is the correct value for the context. I don't mind about the default value too much.

Status & tagging log