11fallingleaves wrote:

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.

11fallingleaves wrote:

(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

• 1. 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

Change 100952 had a related patch set uploaded (by Nemo bis):
Let FauxRequest use data from a Title object

https://gerrit.wikimedia.org/r/100952

Change 100952 abandoned by Thiemo Kreuz (WMDE):

[mediawiki/core@master] Let FauxRequest use data from a Title object

Reason:

7 years old. Untested. In conflict. Tickets closed or staled.

https://gerrit.wikimedia.org/r/100952