Page MenuHomePhabricator

"a.mw-selflink" links miss styling
Closed, ResolvedPublic

Description

As in the desktop version, "a.mw-selflink" links should be styled, namely:

  • color inherit
  • font-weight bold
  • no underline on hover

Currently, these links are quite confusing on the mobile version.

Event Timeline

Od1n created this task.Nov 28 2017, 8:22 AM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
Od1n added a comment.Nov 28 2017, 8:24 AM

Note these links already have a caret on hover, as it should be.

Dbrant added a subscriber: Dbrant.Nov 28 2017, 12:47 PM

Can we have an example article where this is observed?

Jdlrobson added a subscriber: Jdlrobson.

mw-self;oml is provided by https://github.com/wikimedia/mediawiki/blob/b831ba8d771978cf7a6313a6ad3e192912a56533/resources/src/mediawiki.legacy/shared.css (the legacy module). Given it's in legacy code it's not clear if this is deprecated and needs to be styled this way.

More information will be needed showing context of this link and why these are confusing to rectify this.

@Od1n is it possible to see an example article where that markup is used?

Personally I'm not sure I understand the existence of this code and the need to style it differently. Why style it at all? I imagine this is why it's marked as legacy...?

<a class="mw-selflink selflink">Bar</a>

I'd appreciate understanding the history of why this isn't outputted as:

<span>Bar</span>

@TheDJ don't suppose you have any background? Is there any reason we should encourage it?

I got a bit more context about this in the office. It seems one main use case of this is inside Navboxes where the link to the current page is bolded.
Are there other use cases which are hurting readability?

TheDJ added a comment.EditedNov 29 2017, 10:12 PM

yeah it's mostly a safety method to avoid that links stemming from templated transclusions don't get link styling when they point the exact same page you are currently on. The bolding is less important i think.

Navbox is the most common usage for this behavior, but i'm sure there are other.

Oh, and it used to be <strong>Bar</strong>, but the parsing team changed it to <a>.

bmansurov moved this task from Needs triage to Triaged on the Mobile board.Nov 29 2017, 10:17 PM

To avoid confusion with other links, it might be an option to use a[href] or even better a:link instead of just a as the css selector for normal links.

Cirdan added a subscriber: Cirdan.Dec 4 2017, 8:14 AM
Cirdan added a comment.Dec 4 2017, 8:21 AM

The bolding is less important i think.

This feature is used heavily in navigation boxes and menus (such as the one one the right on de:Hilfe:Bilder) and the bold formatting is very important for this to work. If the self links were just plain text with normal weight, it would be much harder to spot them.

While I understand that this use is likely not the original purpose and not very elegant/clean from a developer point of view, it is a neat trick which has been employed for more than a decade and makes it very easy for users to create this kind of navigation.

Change 395173 had a related patch set uploaded (by Gerrit Patch Uploader; owner: Vlakoff):
[mediawiki/skins/MinervaNeue@master] Add missing styles for "a.mw-selflink" links

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

Od1n added a comment.EditedDec 5 2017, 4:23 AM

@Jdlrobson requested more discussion (* I wouldn't have guessed that such an obvious, simple and straightforward issue would require discussion…),

So here is another use case. Several pages, Page A, Page B, etc. that have the same navigation links:

* [[Page A]]
* [[Page B]]
* [[Page C]]

This way, the list is much easier to maintain (copy-paste, template…) and even more importantly, when navigating between pages, the list items stay at the same position.

By the way, such links exist since the beginning and yes, they are well known. Dropping them is obviously out of question. And currently they render as a complete disturbing mess, because styles are obviously missing.

And what's that about "first paint" performance? Do you seriously think there's a bottleneck there? (meanwhile we could instead talk about the shitload amount of javascript code…)

Change 396136 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] POC: Rewrite self links as strong tags

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

And what's that about "first paint" performance? Do you seriously think there's a bottleneck there? (meanwhile we could instead talk about the shitload amount of javascript code…)

First paint is the time from requesting a url to a user seeing something on the screen. Any increase in CSS will negatively impact this metric. JavaScript code is not render blocking so does not impact this. The CSS we load on mobile is half that on desktop as we challenge any addition to CSS. My job involves caring about this. Although this on its own does not merit a problem, if we accepted every requested minor change to CSS that we get we'd have a huge problem.
We track performance here if interested: https://grafana.wikimedia.org/dashboard/db/mobile-2g?orgId=1

So here is another use case

I understand the hypothetical problem I just don't understand where this creates confusion. I'm asking for URLs for actual articles that use this and look broken so I can understand this problem better and understand the trade offs. As I've pointed out there are no navboxes shown on mobile so I'm a little confused about how something we do not display can "render as a complete disturbing mess,".

So far the only example I see is:

(which although confusing is not a disturbing mess)

These styles have been "dropped for 5 years so the fact these have been only surfaced now make me skeptical that they are actually necessary.

What seems like a better solution, is to convert .mw-selflink into strong elements inside the MobileFrontend given we are parsing the page anyhow (per patch). This results in the correct display without needing CSS. Are there any URLs that this technique won't work for? In fact are there good reasons we don't output a strong tag in core in this circumstance?

I'm asking for URLs for actual articles that use this and look broken so I can understand this problem better and understand the trade offs.

Here a user complained that the navigation on this page is practically non-functional on mobile.

These styles have been "dropped for 5 years so the fact these have been only surfaced now make me skeptical that they are actually necessary.

All navigation boxes I've ever seen both on Wikimedia and non-Wikimedia MediaWikis are built using this feature. As far as I'm aware of, there is no other way to achieve the same effect without adding custom CSS and/or JS.

What seems like a better solution, is to convert .mw-selflink into strong elements inside the MobileFrontend given we are parsing the page anyhow (per patch). This results in the correct display without needing CSS. Are there any URLs that this technique won't work for? In fact are there good reasons we don't output a strong tag in core in this circumstance?

If that's the better solution from a technical point of view, I don't see any obvious issues with that. As long as self-links are styled approximately as they are now and this style is consistent across all officially supported Wikimedia skins, any technical solution should do.

Od1n added a comment.Dec 8 2017, 1:57 AM

I think returning a modified, different markup on mobile version is quite surprising, and a wide open door for bugs.
As visible on your Gerrit patch, the markup is litteraly hacked (your patch code is clean though, I'm talking about the principle, of this one, and of the other ones that are already stacked...).

Just as an example, we have a lot of complains on fr.wikipedia because the ".navbox" elements are removed on the mobile version.

Od1n added a comment.Dec 8 2017, 2:02 AM

What I'm suggesting:

  • First, just add my CSS,
  • Then possibly, in a new ticket, transform <a class="selflink"> to <strong class="selflink">, for both desktop and mobile. At the source, not in a post-process layer.
Od1n added a comment.Dec 8 2017, 2:10 AM

Just noticed this: apparently, in the past, the generated elements were <strong class="selflink">, then at some point it has been changed to <a class="selflink">.

The commit where this change was made, and more importantly, the reason, should be found back.

Od1n added a comment.EditedDec 8 2017, 3:15 AM

Here it is: T160480 (gerrit). Also, along this patch the mobile styles were probably forgotten, their omission wasn't on purpose.

(btw, a related discussion: T21213)

Change 396136 abandoned by Jdlrobson:
POC: Rewrite self links as strong tags

Reason:
Was a POC

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

whym added a subscriber: whym.Mar 2 2018, 12:25 PM

I'd like to present a usecase outside of navboxes: at Japanese Wiktionary, we add quotes from literatures to provide examples of the use of a word, as many Wiktionaries do. The appearances of the headword in an example sentence is highlighted by bolding it.

Since one sentence can contain more than one word, it can be reused on different articles. This is where 'bolding by self-linking' is useful - It's convenient to be able to copy and paste and let the headword automatically highlighted.

In https://ja.wiktionary.org/w/index.php?title=%E5%89%B2%E6%84%9B&oldid=804869&veaction=editsource&action=edit and https://ja.wiktionary.org/w/index.php?title=%E7%B4%99%E9%9D%A2&diff=818659&oldid=692359&diffmode=source I reused the same example sentence to highlight different headwords. ("[[貴重]]な[[紙面]]の御[[割愛]]を[[ねがう|願ふ]]ことを御許し下さい。")

Od1n added a comment.Mar 2 2018, 1:16 PM

Thank you for this addition. But as I stated a bit above, these missing styles are simply an overlook from T160480. There is just no reason not to add them.

Do we have a clear path forward, however? It looks like we have consensus on that self-links should be styled as proposed (bold, inherited color, etc). But there is a disagreement over how to implement the desired style: 1) using <a> and add CSS rules, or 2) using <strong>. And it looks like from performance perspectives <strong> is preferable, while semantically <a> should make more sense.

To me, both seem to have their own reasons. I wonder if @Jdforrester-WMF can share insights?

In the meantime, I'm going to use an on-wiki workaround.

TheDJ added a comment.Mar 5 2018, 6:47 AM

Added workaround to en.wp as well.

Od1n added a comment.EditedMar 5 2018, 8:23 PM

This needs more than the single rule above. I had to add the following to fr.wiki: https://fr.wikipedia.org/w/index.php?title=MediaWiki:Mobile.css&diff=prev&oldid=143649876

(to summarize, it's these changes: 343204 plus 345525)

Change 416733 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/core@master] Self links should not be marked as legacy css

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

Change 416733 merged by jenkins-bot:
[mediawiki/core@master] Self links should not be marked as legacy css

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

Od1n added a comment.Mar 8 2018, 3:28 AM

Just to clarify, the patch that has just been merged is not for the issue of this ticket, but for a related issue.

To avoid confusion with other links, it might be an option to use a[href] or even better a:link instead of just a as the css selector for normal links.

After a long think about this I think this is the way forward. Only browser it won't work in is IE6 but Minerva is specifically not designed for that browser.

I notice this is the default behaviour for the chrome default stylesheet.

The other styles Minerva would inherit from reusing the core implementation is not justified given how much it deviates and adding/using the self link class feels very 2001.

I'll get a patch up.

Change 421956 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@master] Do not style links without href attribute

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

Jdlrobson moved this task from Inbox to Blocked on the User-Jdlrobson board.Mar 26 2018, 5:45 PM

Change 421956 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Do not style links without href attribute

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

Jdlrobson closed this task as Resolved.Mar 27 2018, 6:47 PM
Jdlrobson claimed this task.

This will roll out on next weeks train (Thur 4th)

Change 422442 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@master] Last modified links should be white

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

Change 422442 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Last modified links should not be progressive blue

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

whym added a comment.EditedApr 10 2018, 7:41 AM

Is there anything that local wikis have to do after the change? Japanese Wiktionary (still) has CSS rules for self links in https://ja.wiktionary.org/wiki/MediaWiki:Mobile.css but it doesn't seem to be applied to https://ja.m.wiktionary.org/wiki/%E3%81%AA%E3%81%8D%E3%82%80%E3%81%97 like they used to be. The intended targets on that page include the first appearance of 泣き虫 which is not bolded any more. (underlines appear when I hover the pointer over them, though.)

EDIT: it looks like all rules on Mobile.css are ignored. https://ja.m.wiktionary.org/w/index.php?title=忖度&oldid=1039744 has dd elements that should get a prefix "▸ " because of the first rule in Mobile.css, but they don't. Is it perhaps unrelated to self-links? Sorry if that's the case.

Jdlrobson added a comment.EditedApr 10 2018, 2:03 PM

Hi @whym see https://phabricator.wikimedia.org/T191596 - we are looking into it and there is a workaround suggested there. No styling rules should be needed to support self links. Sorry for the disruption!!

whym awarded a token.Apr 10 2018, 2:10 PM

Thanks - the workaround suggested there seems to work for me for the moment.

Change 395173 abandoned by Jdlrobson:
Add missing styles for "a.mw-selflink" links

Reason:
This is no longer needed. We've updated the styles to account for self links without needing to refer to the mw-selflink (I61b05e3c223f2ba5314aecdf26b8a0ee8caa6524)

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

Od1n removed a subscriber: Od1n.Aug 2 2018, 3:49 AM