Page MenuHomePhabricator

Show self links in bold font
Closed, ResolvedPublic

Description

In MediaWiki, self links (links to the page itself) are rendered as normal bold text (without a link). In cargo, if a table contains a link to the page itself, it is rendered as any other link. I think it would be good to follow the MediaWiki style and check for self links to render them as bold text. I have been looking into this and this patch solves the problem:

diff --git a/includes/CargoUtils.php b/includes/CargoUtils.php
index 91a935f..abcaff6 100644
--- a/includes/CargoUtils.php
+++ b/includes/CargoUtils.php
@@ -1143,6 +1143,17 @@ class CargoUtils {
        public static function makeLink( $linkRenderer, $title, $msg = null, $attrs = array(), $params = array() ) {
                if ( is_null( $title ) ) {
                        return null;
+               }
+
+               $ctx = RequestContext::getMain();
+               $tempTitle = $ctx->getTitle();
+
+               if ( !is_null( $tempTitle ) && $title->getArticleId() == $tempTitle->getArticleId() ) {
+                       if ( is_null( $msg ) ) {
+                               return Html::element('strong', $attrs, $title->getBaseText());
+                       } else {
+                               return Html::element('strong', $attrs, $msg);
+                       }
                } elseif ( !is_null( $linkRenderer ) ) {
                        // MW 1.28+
                        // Is there a makeLinkKnown() method? We'll just add the

To obtain the current page, the context is obtained through the RequestContext::getMain() function. However, the MediaWiki documentation about this function (https://www.mediawiki.org/wiki/Manual:RequestContext.php) says that it should be used as a last resort, so I don't know if it is better to modify the function signature to include the context as a parameter.

Event Timeline

Yaron_Koren claimed this task.
Yaron_Koren added a subscriber: Yaron_Koren.

@Tombolano - Thanks for this bug report, and patch; I didn't realize that self-links were not being handled in the right way. I just checked in a patch based on this one - not quite the same code, but the same idea.

@Tombolano Thanks for this and for your other recent contribution (about HOLDS). I had worked round this one using CSS as I wrongly thought it was a Foreground skin problem. I'm "watching" this project now in the hope that you continue your interest :-)

Thanks for the fix Yaron!, certainly your code is better than mine, I do not have experience with MediaWiki coding. However, you forgot to check $wgTitle for null and now the mainteinance scripts fail (at least cargoRecreateData), yikes! I am uploading a patch to fix it.

Change 510987 had a related patch set uploaded (by Tombolano; owner: Tombolano):
[mediawiki/extensions/Cargo@master] Fix for non-checked null value in makeLink

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

Change 510987 merged by jenkins-bot:
[mediawiki/extensions/Cargo@master] Fix for non-checked null value in makeLink

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

I'm closing this since the fix has been merged. Now everything works fine. Although one of the patch reviewers said that there is a ->equals() method in the Title class and that maybe we should use it.

Sorry about that problem, and thanks for the quick fix! I just checked in another fix involving equals() - it seems to work better in certain cases. Hopefully everything works now.