r75617 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r75616‎ | r75617 | r75618 >
Date:16:58, 28 October 2010
Author:philip
Status:ok (Comments)
Tags:
Comment:
1. Revert the complicated redirection I made in r59754;
2. Add more Accept-Language XVO cache name for IE;
3. Use hreflang to specify canonical and alternate links, it's search engine friendly
when a wiki has multiple variant languages.
Modified paths:
  • /trunk/phase3/RELEASE-NOTES (modified) (history)
  • /trunk/phase3/includes/DefaultSettings.php (modified) (history)
  • /trunk/phase3/includes/OutputPage.php (modified) (history)
  • /trunk/phase3/includes/Skin.php (modified) (history)
  • /trunk/phase3/includes/Title.php (modified) (history)
  • /trunk/phase3/includes/User.php (modified) (history)
  • /trunk/phase3/includes/Wiki.php (modified) (history)
  • /trunk/phase3/languages/Language.php (modified) (history)
  • /trunk/phase3/languages/LanguageConverter.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/User.php
——@@ -1024,7 +1024,7 @@
10251025 /**
10261026 * default language setting
10271027 */
1028 - $variant = $wgContLang->getPreferredVariant( false );
 1028+ $variant = $wgContLang->getDefaultVariant();
10291029 $defOpt['variant'] = $variant;
10301030 $defOpt['language'] = $variant;
10311031 foreach( SearchEngine::searchableNamespaces() as $nsnum => $nsname ) {
Index: trunk/phase3/includes/OutputPage.php
——@@ -1430,7 +1430,17 @@
14311431 if( $variant === $wgContLang->getCode() ) {
14321432 continue;
14331433 } else {
1434 - $aloption[] = "string-contains=$variant";
 1434+ $aloption[] = 'string-contains=' . $variant;
 1435+
 1436+ // IE and some other browsers use another form of language code
 1437+ // in their Accept-Language header, like "zh-CN" or "zh-TW".
 1438+ // We should handle these too.
 1439+ $ievariant = explode( '-', $variant );
 1440+ if ( count( $ievariant ) == 2 ) {
 1441+ $ievariant[1] = strtoupper( $ievariant[1] );
 1442+ $ievariant = implode( '-', $ievariant );
 1443+ $aloption[] = 'string-contains=' . $ievariant;
 1444+ }
14351445 }
14361446 }
14371447 $this->addVaryHeader( 'Accept-Language', $aloption );
Index: trunk/phase3/includes/Wiki.php
——@@ -208,19 +208,11 @@
209209 throw new ErrorPageError( 'badtitle', 'badtitletext' );
210210 }
211211 // Redirect loops, no title in URL, $wgUsePathInfo URLs, and URLs with a variant
212 - } else if( $action == 'view' && !$request->wasPosted() &&
213 - ( ( !$request->getVal( 'title' ) || $title->getPrefixedDBKey() != $request->getText( 'title' ) ) ||
214 - // No valid variant in URL (if the main-language has multi-variants), to ensure
215 - // anonymous access would always be redirect to a URL with 'variant' parameter
216 - ( !$request->getVal( 'variant' ) && $wgContLang->hasVariants() && !$wgUser->isLoggedIn() ) ) &&
217 - !count( array_diff( array_keys( $request->getValues() ), array( 'action', 'title' ) ) ) )
 212+ } else if( $action == 'view' && !$request->wasPosted()
 213+ && ( ( !$request->getVal( 'title' ) || $title->getPrefixedDBKey() != $request->getText( 'title' ) ) )
 214+ && !count( array_diff( array_keys( $request->getValues() ), array( 'action', 'title' ) ) ) )
218215 {
219 - if( !$wgUser->isLoggedIn() ) {
220 - $pref = $wgContLang->getPreferredVariant( false, $fromHeader = true );
221 - $targetUrl = $title->getFullURL( '', $variant = $pref );
222 - }
223 - else
224 - $targetUrl = $title->getFullURL();
 216+ $targetUrl = $title->getFullURL();
225217 // Redirect to canonical url, make it a 301 to allow caching
226218 if( $targetUrl == $request->getFullRequestURL() ) {
227219 $message = "Redirect loop detected!\n\n" .
Index: trunk/phase3/includes/Title.php
——@@ -869,14 +869,6 @@
870870 $query = wfArrayToCGI( $query );
871871 }
872872
873 - // internal links should point to same variant as current page (only anonymous users)
874 - if ( !$variant && $wgContLang->hasVariants() && !$wgUser->isLoggedIn() ) {
875 - $pref = $wgContLang->getPreferredVariant( false );
876 - if ( $pref != $wgContLang->getCode() ) {
877 - $variant = $pref;
878 - }
879 - }
880 -
881873 if ( $this->isExternal() ) {
882874 $url = $this->getFullURL();
883875 if ( $query ) {
Index: trunk/phase3/includes/DefaultSettings.php
——@@ -1971,6 +1971,9 @@
19721972 /** Whether to enable language variant conversion for links. */
19731973 $wgDisableTitleConversion = false;
19741974
 1975+/** Whether to enable language cononical in meta data. */
 1976+$wgDisableLangCanonical = false;
 1977+
19751978 /** Default variant code, if false, the default will be the language code */
19761979 $wgDefaultLanguageVariant = false;
19771980
Index: trunk/phase3/includes/Skin.php
——@@ -249,12 +249,23 @@
250250 }
251251
252252 /**
253 - * Adds metadata links (Creative Commons/Dublin Core/copyright) to the HTML
254 - * output.
 253+ * Adds metadata links below to the HTML output.
 254+ * <ol>
 255+ * <li>Creative Commons
 256+ * <br />See http://wiki.creativecommons.org/Extend_Metadata.
 257+ * </li>
 258+ * <li>Dublin Core</li>
 259+ * <li>Use hreflang to specify canonical and alternate links
 260+ * <br />See http://www.google.com/support/webmasters/bin/answer.py?answer=189077
 261+ * </li>
 262+ * <li>Copyright</li>
 263+ * <ol>
 264+ *
255265 * @param $out Object: instance of OutputPage
256266 */
257267 function addMetadataLinks( OutputPage $out ) {
258268 global $wgEnableDublinCoreRdf, $wgEnableCreativeCommonsRdf;
 269+ global $wgDisableLangConversion, $wgDisableLangCanonical, $wgContLang;
259270 global $wgRightsPage, $wgRightsUrl;
260271
261272 if ( $out->isArticleRelated() ) {
——@@ -275,6 +286,29 @@
276287 );
277288 }
278289 }
 290+
 291+ if ( !$wgDisableLangConversion && !$wgDisableLangCanonical
 292+ && $wgContLang->hasVariants() ) {
 293+
 294+ $urlvar = $wgContLang->getURLVariant();
 295+
 296+ if ( !$urlvar ) {
 297+ $variants = $wgContLang->getVariants();
 298+ foreach ( $variants as $_v ) {
 299+ $out->addLink( array(
 300+ 'rel' => 'alternate',
 301+ 'hreflang' => $_v,
 302+ 'href' => $this->mTitle->getLocalURL( '', $_v ) )
 303+ );
 304+ }
 305+ } else {
 306+ $out->addLink( array(
 307+ 'rel' => 'canonical',
 308+ 'href' => $this->mTitle->getFullURL() )
 309+ );
 310+ }
 311+ }
 312+
279313 $copyright = '';
280314 if ( $wgRightsPage ) {
281315 $copy = Title::newFromText( $wgRightsPage );
Index: trunk/phase3/languages/LanguageConverter.php
——@@ -134,20 +134,18 @@
135135
136136 /**
137137 * Get preferred language variant.
138 - * @param $fromUser Boolean: get it from $wgUser's preferences
139 - * @param $fromHeader Boolean: get it from Accept-Language
140138 * @return String: the preferred language code
141139 */
142 - public function getPreferredVariant( $fromUser = true, $fromHeader = false ) {
143 - global $wgDefaultLanguageVariant;
 140+ public function getPreferredVariant() {
 141+ global $wgDefaultLanguageVariant, $wgUser;
144142
145143 $req = $this->getURLVariant();
146144
147 - if ( $fromUser && !$req ) {
 145+ if ( $wgUser->isLoggedIn() && !$req ) {
148146 $req = $this->getUserVariant();
149147 }
150148
151 - if ( $fromHeader && !$req ) {
 149+ elseif ( !$req ) {
152150 $req = $this->getHeaderVariant();
153151 }
154152
——@@ -166,6 +164,26 @@
167165 }
168166
169167 /**
 168+ * Get default variant.
 169+ * This function would not be affected by user's settings or headers
 170+ * @return String: the default variant code
 171+ */
 172+ public function getDefaultVariant() {
 173+ global $wgDefaultLanguageVariant;
 174+
 175+ $req = $this->getURLVariant();
 176+
 177+ if ( $wgDefaultLanguageVariant && !$req ) {
 178+ $req = $this->validateVariant( $wgDefaultLanguageVariant );
 179+ }
 180+
 181+ if ( $req ) {
 182+ return $req;
 183+ }
 184+ return $this->mMainLanguageCode;
 185+ }
 186+
 187+ /**
170188 * Validate the variant
171189 * @param $variant String: the variant to validate
172190 * @return Mixed: returns the variant if it is valid, null otherwise
——@@ -183,7 +201,7 @@
184202 *
185203 * @return Mixed: variant if one found, false otherwise.
186204 */
187 - protected function getURLVariant() {
 205+ public function getURLVariant() {
188206 global $wgRequest;
189207
190208 if ( $this->mURLVariant ) {
Index: trunk/phase3/languages/Language.php
——@@ -44,6 +44,8 @@
4545 function convertTitle( $t ) { return $t->getPrefixedText(); }
4646 function getVariants() { return array( $this->mLang->getCode() ); }
4747 function getPreferredVariant() { return $this->mLang->getCode(); }
 48+ function getDefaultVariant() { return $this->mLang->getCode(); }
 49+ function getURLVariant() { return ''; }
4850 function getConvRuleTitle() { return false; }
4951 function findVariantLink( &$l, &$n, $ignoreOtherCond = false ) { }
5052 function getExtraHashOptions() { return ''; }
——@@ -2673,9 +2675,17 @@
26742676 return $this->mConverter->getVariants();
26752677 }
26762678
2677 - function getPreferredVariant( $fromUser = true, $fromHeader = false ) {
2678 - return $this->mConverter->getPreferredVariant( $fromUser, $fromHeader );
 2679+ function getPreferredVariant() {
 2680+ return $this->mConverter->getPreferredVariant();
26792681 }
 2682+
 2683+ function getDefaultVariant() {
 2684+ return $this->mConverter->getDefaultVariant();
 2685+ }
 2686+
 2687+ function getURLVariant() {
 2688+ return $this->mConverter->getURLVariant();
 2689+ }
26802690
26812691 /**
26822692 * If a language supports multiple variants, it is
Index: trunk/phase3/RELEASE-NOTES
——@@ -186,6 +186,8 @@
187187 is usually to set it to -1 to disable the limit)
188188 * (bug 25397) Allow uploading (not displaying) of WebP images, disabled by default
189189 * (bug 23194) Special:ListFiles now has thumbnails
 190+* Use hreflang to specify canonical and alternate links, search engine friendly
 191+ when a wiki has multiple variant languages.
190192
191193 === Bug fixes in 1.17 ===
192194 * (bug 17560) Half-broken deletion moved image files to deletion archive

Follow-up revisions

RevisionCommit summaryAuthorDate
r75618Follow up r75617. remove extra brackets.philip17:11, 28 October 2010
r75639Fix the LanguageConverterTest to prevent test failures after the changes made...philip02:55, 29 October 2010
r75692Forgot to commit this one. parserTests fix for ...philip05:42, 30 October 2010
r75719Followup r75617. Fix spelling mistake.philip15:09, 31 October 2010

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r59754follow-up r59522 and r59735. only redirect to a variant URL when logged out.philip05:39, 5 December 2009

Comments

#Comment by MaxSem (talk | contribs)   18:01, 28 October 2010

Broke unit tests: http://ci.tesla.usability.wikimedia.org/cruisecontrol/buildresults/mw

#Comment by PhiLiP (talk | contribs)   02:02, 29 October 2010

I have reverted the change (add $fromHeader) I made on getPreferredVariant() in r59735, and split $fromUser=false from it to getDefaultVariant(). A fix of the Tester would solve such problem. I'll do it.

#Comment by MaxSem (talk | contribs)   06:20, 29 October 2010

All tests pass, resetting to new.

#Comment by Platonides (talk | contribs)   23:37, 29 October 2010

Breaks parser tests:

  • Running test Prevent conversion of links with -{}- tags (language variants)... FAILED!
--- mwParser-707476044-expected	2010-10-30 01:34:48.000000000 +0200
+++ mwParser-707476044-actual	2010-10-30 01:34:48.000000000 +0200
@@ -1,2 +1,2 @@
-<p><a href="https://www.mediawiki.org/index.php?title=Main_Page&variant=sr-ec" title="Main Page">Main Page</a>
+<p><a href="https://www.mediawiki.org/wiki/Main_Page" title="Main Page">Main Page</a>
 </p>

r75617 fails

Latest good revision: r75616

Earliest bad revision: r75617

#Comment by PhiLiP (talk | contribs)   05:44, 30 October 2010

Also fixed.

#Comment by Nikerabbit (talk | contribs)   09:56, 31 October 2010

+/** Whether to enable language cononical in meta data. */ 

+$wgDisableLangCanonical = false;

Spelling mistake. Should also clarify the description more. The variable name could be positive instead of negative, such as $wgCanonicalLanguageLinks or something.

#Comment by PhiLiP (talk | contribs)   15:10, 31 October 2010

r75719.

Status & tagging log