Last modified: 2011-08-22 10:26:20 UTC
Created attachment 8777 [details] Patch When using ApprovedRevs extension, all pages configured for the extension are always tagged with "noindex,nofollow". Trivial patch attached that fixes this.
Comment on attachment 8777 [details] Patch >--- Article.php.orig 2011-07-13 10:02:31.727795843 +0300 >+++ Article.php 2011-07-13 10:02:21.657795857 +0300 >@@ -1127,7 +1127,7 @@ > } > } > >- if ( $this->getID() === 0 || $this->getOldID() ) { >+ if ( $this->getID() === 0 || $this->getOldID() != $this->mLatest) { > # Non-articles (special pages etc), and old revisions > return array( > 'index' => 'noindex', >
In order to make this small change easier to review, could you add a comment to the patch saying when getOldID would return a true value != mLatest?
It already says that, right below the line: "# Non-articles (special pages etc), and old revisions" This patch just slightly improves the logic, so that simply having mOldID does not automatically indicate that an old revision is being viewed.
Assigning to myself, as the author of Approved Revs.
(In reply to comment #3) > It already says that, right below the line: > "# Non-articles (special pages etc), and old revisions" > > This patch just slightly improves the logic, so that simply having mOldID does > not automatically indicate that an old revision is being viewed. Yes, it does not solve the problem. As a workaround I put : For any casual person who visited this bug, like me today, I changed in Article.php: if ( $this->getID() === 0 ) { } and then I patched my skin to look at oldid and action in URL for adding noindex,nofollow.
I just finally looked into this patch, and realized that it's a patch for core MediaWiki, not the Approved Revs extension - which explains why various people have been commenting on it. Perhaps this should be assigned to someone else, then? And renamed? (It was my mistake for assigning it to myself in the first place.)
(In reply to comment #5) > For any casual person who visited this bug, like me today, I changed in > Article.php: > > if ( $this->getID() === 0 ) { > > } > and then I patched my skin to look at oldid and action in URL for adding > noindex,nofollow. Could you clarify this? I'd like to apply this fix, but I don't quite understand your fix.
(In reply to comment #7) > (In reply to comment #5) > > For any casual person who visited this bug, like me today, I changed in > > Article.php: > > > > if ( $this->getID() === 0 ) { > > > > } > > and then I patched my skin to look at oldid and action in URL for adding > > noindex,nofollow. > > Could you clarify this? I'd like to apply this fix, but I don't quite > understand your fix. Hello Mark, my fix is a workaround and not clean. I just added a parsing of $_SERVER['REQUEST_URI'] against 'action' and 'oldid' in my custom skin and I add meta tag then. Let me see if I can find a cleaner way.
I don't quite see what the problem is with my original patch.
(In reply to comment #9) > I don't quite see what the problem is with my original patch. If I don't remind badly when I tried myself, it adds noindex in those cases that the actual approved page is not the latest one.
(In reply to comment #10) > (In reply to comment #9) > > I don't quite see what the problem is with my original patch. > > If I don't remind badly when I tried myself, it adds noindex in those cases > that the actual approved page is not the latest one. The patch doesn't do that; it just doesn't fix it, either. I'm not familiar enough with mediawiki code to come up with a clean fix for that.
(In reply to comment #11) > (In reply to comment #10) > > (In reply to comment #9) > > > I don't quite see what the problem is with my original patch. > > > > If I don't remind badly when I tried myself, it adds noindex in those cases > > that the actual approved page is not the latest one. > > The patch doesn't do that; it just doesn't fix it, either. I'm not familiar > enough with mediawiki code to come up with a clean fix for that. I would dare to say that your patch works as far as the approved revision is the latest one. I must say I'm not familiar enough either with MW guts. I will try to find some time to learn, though, if noone else can find a quickier solution.
