Page MenuHomePhabricator

Invisible link to notifications can overlap link to personal talk page
Closed, ResolvedPublic

Description

See this screenshot:

Hovering over the link to my talkpage in Monobook skin shows, that I actually hover over the link for the notifications.
This is probably caused by rECHOd4d325e7e68f: Use words for describing notification counts in HTML text node. It's true that these links aren't visible (though this might not always be true, I found this issue while trying to reproduce a report by a user who has these links visibly overlapping other stuff, https://de.wikipedia.org/wiki/Wikipedia_Diskussion:Projektneuheiten#Icons_fehlen.3F), they definitely are there and can overlap other links.

Event Timeline

Schnark created this task.Mar 24 2017, 11:24 AM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript

Also reproducible in Vector, though (at least with German localization) the overlapping area is much smaller (only parts of the "D" of "Diskussion").

Are they visibility:hidden or color:transparent?

#pt-notifications-alert .mw-echo-notifications-badge,
#pt-notifications-notice .mw-echo-notifications-badge {
 color: transparent;
}

according to the CSS inspector.

I still don't understand why some users actually see these links, but one way to reproduce is to force the browser to use its own color for links (in Firefox: Settings > Content > Colors > Overrride the colors: Always).

The link was originally visibility: hidden but it was made color: transparent as part of a change @matmarex made. Both of them are hacks, though, because the notification badge (which does need to be visible) is the :before / :after of that link. Rather than using color: transparent for the link and overriding the color on the :before/:after (which was more tolerable when the content was just a number, not text translated into languages with long words), we should probably go back to the technique of setting visibility: visible on the link and overriding it to visibility: visible on the :before/:after.

color: transparent is mine (de795bdbc5ee928d570c58ccb6d7b803dfa9504f), but previously it was font-size: 0 (which didn't work for Opera 12, as it enforces a minimum font size). I'm not sure if we ever tried using visibility: hidden.

Oh that's right, we used font-size: 0, I forget who came up with that but @Mooeypoo might remember. Not sure why we didn't go with the nested visibility thing, it seems to work, but maybe there are browser issues with that too? Or maybe we were just afraid of them?

Actually, visibility: hidden probably wouldn't be okay, since it would hide the content from screen-readers. But we can probably do position: relative; top: -999px; for the element and position: absolute; top: 999px; for the pseudoelements?

I considered positioning off-screen and rejected it because of the :before / :after thing, but I guess if we make the high numbers cancel out then it should be fine. I'll try that.

I'm pretty sure I remember that @Volker_E has created a mixin specifically for these kinds of strings.

Yes, we have .mixin-screen-reader-text(), but that won't work well here. To elaborate, we have a structure like this (which would be difficult to change because of skin limitations):

<a href="/wiki/Special:Notifications" class="mw-echo-notifications-badge ..." ...>
  <::before />
  Alerts (0)
  <::after />
</a>

We want to hide the "Alerts (0)", but show the ::before (which creates the badge icon) and ::after (which creates the number).

.mixin-screen-reader-text() works by making the element dimensions 1x1px and hiding overflow – there's no way for child elements (including pseudoelements) to escape that.

Note: For testing after the fix - the both tooltips are presently visible depending on the position of the pointer (the tooltip is not entirely missing). The issue is not specific to German or to monobok skin.

Change 345173 had a related patch set uploaded (by Catrope):
[mediawiki/extensions/Echo@master] Position badge text off-screen instead of making it transparent

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

Change 345176 had a related patch set uploaded (by Bartosz Dziewoński):
[mediawiki/extensions/Echo@master] Make the invisible text in badges really invisible

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

Uh, whoops, apparently I decided to work on this at exactly the same time. Please ignore me.

Change 345176 abandoned by Bartosz Dziewoński:
Make the invisible text in badges really invisible

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

Change 345176 restored by Catrope:
Make the invisible text in badges really invisible

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

Change 345173 abandoned by Catrope:
Position badge text off-screen instead of making it transparent

Reason:
https://gerrit.wikimedia.org/r/#/c/345176

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

Uh, whoops, apparently I decided to work on this at exactly the same time. Please ignore me.

That's OK, your patch was better anyway, so I abandoned mine and +2ed yours.

Change 345176 merged by jenkins-bot:
[mediawiki/extensions/Echo@master] Make the invisible text in badges really invisible

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

Checked the fix in betalabs (with monobook skin too).

QA Recommendation:Resolve

TheDJ awarded a token.Mar 29 2017, 7:52 AM
jmatazzoni closed this task as Resolved.Mar 29 2017, 8:19 PM