Last modified: 2013-12-17 09:53:29 UTC
This is what I get when setting $wgEnableCanonicalServerLink to true in LocalSettings.php: $ php maintenance/rebuildFileCache.php Building content page file cache from page 0! Page 1 already cached [057522cb] [no req] Exception from line 1334 of /home/<username>/public_html/mediawiki/includes/WebRequest.php: FauxRequest::getRequestURL() not implemented Backtrace: #0 /home/<username>/public_html/mediawiki/includes/WebRequest.php(1385): FauxRequest->notImplemented(string) #1 /home/<username>/public_html/mediawiki/includes/OutputPage.php(3298): FauxRequest->getRequestURL() #2 /home/<username>/public_html/mediawiki/includes/OutputPage.php(3316): OutputPage->getHeadLinksArray() #3 /home/<username>/public_html/mediawiki/includes/OutputPage.php(2518): OutputPage->getHeadLinks() #4 /home/<username>/public_html/mediawiki/includes/SkinTemplate.php(498): OutputPage->headElement(SkinVector) #5 /home/<username>/public_html/mediawiki/includes/OutputPage.php(2078): SkinTemplate->outputPage() #6 /home/<username>/public_html/mediawiki/maintenance/rebuildFileCache.php(135): OutputPage->output() #7 /home/<username>/public_html/mediawiki/maintenance/doMaintenance.php(113): RebuildFileCache->execute() #8 /home/<username>/public_html/mediawiki/maintenance/rebuildFileCache.php(163): require_once(string) #9 {main} This change fixes the bug for me, but it seems kind of hackish: diff --git a/includes/WebRequest.php b/includes/WebRequest.php index b17cb9e..05ffae7 100644 --- a/includes/WebRequest.php +++ b/includes/WebRequest.php @@ -1382,7 +1382,8 @@ class FauxRequest extends WebRequest { } public function getRequestURL() { - $this->notImplemented( __METHOD__ ); + global $wgTitle; + return $wgTitle->getLocalURL(); } /** It works, though: $ php maintenance/rebuildFileCache.php Building content page file cache from page 0! Cached page 1 Cached page 2 Done! How should I submit this change?
This is on branch master and REL1_22.
(In reply to comment #0) > > This change fixes the bug for me, but it seems kind of hackish: > diff --git a/includes/WebRequest.php b/includes/WebRequest.php > index b17cb9e..05ffae7 100644 > --- a/includes/WebRequest.php > +++ b/includes/WebRequest.php > @@ -1382,7 +1382,8 @@ class FauxRequest extends WebRequest { > } > > public function getRequestURL() { > - $this->notImplemented( __METHOD__ ); > + global $wgTitle; > + return $wgTitle->getLocalURL(); > } > > /** > > It works, though: > > $ php maintenance/rebuildFileCache.php > Building content page file cache from page 0! > Cached page 1 > Cached page 2 > Done! > > How should I submit this change? Sorry, but that's probably going to be too hacky to be accepted. ($wgTitle is not allowed to be used in new code). I'm not entirely sure why we're outputting the WebRequest::getRequestUrl as the canonical url. Maybe it might make sense to change the OutputPage::getHeadLinksArray method to use $this->getTitle()->getLocalURL(); instead of $this->getRequest()->getRequestURL(). (I haven't looked at the code very much at all, but intuitively, the canonical link shouldn't have extra junk from the url anyway) ---- For reference, you can submit patches to our gerrit (https://gerrit.wikimedia.org) install. Accounts are free, and available by signing up at http://wikitech.wikimedia.org. There's detailed instructions at https://www.mediawiki.org/wiki/Gerrit/Tutorial . Alternatively you can upload a patch via http://tools.wmflabs.org/gerrit-patch-uploader/ without having to go through all the git set up.
(In reply to comment #2) > Sorry, but that's probably going to be too hacky to be accepted. ($wgTitle is > not allowed to be used in new code). Ok, of course. (For me it was a hack to get things working, so I didn't care). > I'm not entirely sure why we're outputting the WebRequest::getRequestUrl as > the > canonical url. Maybe it might make sense to change the > OutputPage::getHeadLinksArray method to use $this->getTitle()->getLocalURL(); > instead of $this->getRequest()->getRequestURL(). (I haven't looked at the > code > very much at all, but intuitively, the canonical link shouldn't have extra > junk > from the url anyway) Seems like a good idea. I did a few changes which seem to work fine, but my MediaWiki installation is broken so I can't properly test right now. I also changed something else (DRY), so it looks better to me. I don't really like the variable re-use that's going on there. First, $canonicalUrl is used as a relative URL and later it's re-used as an absolute URL. diff --git a/includes/OutputPage.php b/includes/OutputPage.php index f1c7d5b..4238776 100644 --- a/includes/OutputPage.php +++ b/includes/OutputPage.php @@ -3297,12 +3297,10 @@ $templates # Canonical URL global $wgEnableCanonicalServerLink; if ( $wgEnableCanonicalServerLink ) { - if ( $canonicalUrl !== false ) { - $canonicalUrl = wfExpandUrl( $canonicalUrl, PROTO_CANONICAL ); - } else { - $reqUrl = $this->getRequest()->getRequestURL(); - $canonicalUrl = wfExpandUrl( $reqUrl, PROTO_CANONICAL ); + if ( $canonicalUrl === false ) { + $canonicalUrl = $this->getTitle()->getLocalURL(); } + $canonicalUrl = wfExpandUrl( $canonicalUrl, PROTO_CANONICAL ); } if ( $canonicalUrl !== false ) { $tags[] = Html::element( 'link', array( Note, though, that Google recommends the canonical link to be absolute (this is the case with this change): https://support.google.com/webmasters/answer/139394?hl=en I think that might also be better when a wiki can be reached from multiple domain names or both HTTP and HTTPS. Thanks for the git/gerrit information :) If I have the time I will properly test and submit a patch.
Change 100952 had a related patch set uploaded by Leaves in Motion: Use Title instead of Request for the canonical url https://gerrit.wikimedia.org/r/100952
