Page MenuHomePhabricator

Title attributes not properly removed on hover
Closed, ResolvedPublic3 Story Points

Description

Problems with mw.popups.removeTooltip (multiple problems):

  • title attribute of links are not removed on focus, causing qunit test failure (locally only, for some reason)
    • Repro steps:
      1. Run qunit tests locally, observe failure of 3rd assertion in ext.popups.core: removeTooltips test. Alternatively, try manually tabbing focus onto a link with popups enabled.
  • Sometimes on mouseover the title attribute is not removed at all.
    • Repro steps:
      1. Go to https://en.wikipedia.beta.wmflabs.org/wiki/25thjuneChrome (fresh page reload)
      2. Inspect first link ("List of books about Wikipedia"), observe title attribute is present.
      3. Hover over link, observe title attribute is not removed. Repeat steps 1-3 if it is removed. Cannot find consistent manner in which to reproduce, but it happens frequently.
  • The title attribute is sometimes not restored on mouseout, breaking it for the rest of the page session.
    • Repro steps:
      1. Go to https://en.wikipedia.beta.wmflabs.org/wiki/25thjuneChrome (fresh page reload)
      2. Inspect first link ("List of books about Wikipedia"), observe title attribute is present.
      3. Hover over link, observe title attribute is properly removed. (Repeat step if 2nd problem above is encountered.)
      4. Mouse off of link, observe title attribute is properly restored. (Usually is on first try, anyways.)
      5. Repeat steps 3-4, observe title attribute is not restored. Cannot find consistent manner in which to reproduce, so repeat steps 3-4 until it happens.

Note that none of these problems appear to be race conditions. They all occur even if you wait for scripts to fully load and animations to complete.


Story
As a reader, I don't want to see both the title of the article and the hovercard, as it is distracting

Acceptance Criteria
Title tag doesn't appear when hovercards are on
Title tag appears when hovercards are off

Description

MediaWiki sets the title tag on all internal links, and many/most browsers show it as a small popup when one hovers over the link. This often interfers with hovercards (not always - I'm also seeing lots of hovercards without accompanying title popups).

Example screenshots

from https://hu.wikipedia.org/wiki/James_Bond , browsing anonymously on September 10:

from https://de.wikipedia.org/wiki/Wikipedia:Hauptseite , browsing while logged in with Hovercards beta feature enabled on August 17:

from https://de.wikipedia.org/wiki/Cannes , browsing while logged in with Hovercards beta feature enabled on August 17:

All on Chrome under Linux.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 11 2016, 3:55 PM
jhobs updated the task description. (Show Details)Aug 11 2016, 4:05 PM
jhobs updated the task description. (Show Details)
jhobs triaged this task as Normal priority.Aug 11 2016, 5:17 PM
jhobs moved this task from Incoming to Needs Prioritization on the Readers-Web-Backlog board.
jhobs moved this task from Needs Prioritization to Incoming on the Readers-Web-Backlog board.
jhobs added a project: Technical-Debt.
jhobs moved this task from Incoming to Triaged but Future on the Readers-Web-Backlog board.
jhobs raised the priority of this task from Normal to High.Sep 14 2016, 4:13 PM
Jhernandez renamed this task from mw.popups.removeTooltips is not functioning properly (multiple problems) to Title attributes not properly removed on hover.Sep 14 2016, 4:13 PM
Jhernandez updated the task description. (Show Details)

Seems to happen in firefox and other browsers too osx/linux, probably everywhere.

Seems to happen in firefox and other browsers too osx/linux, probably everywhere.

I believe it's everywhere. I observed the problem on Chrome on Win7 and OSX.

Removing Technical-Debt as this has been elevated to a proper bug.

ovasileva updated the task description. (Show Details)Sep 23 2016, 3:55 PM
MBinder_WMF set the point value for this task to 3.Sep 26 2016, 4:29 PM

@MBinder_WMF I'm unsure of whether to add Unplanned-Sprint-Work to this task as during kickoff we did plan a couple tasks to pull in (and this was one of them).

jhobs moved this task from Needs Analysis to Doing on the Reading-Web-Sprint-83-Y? board.
Jdlrobson added a subscriber: Jdlrobson.

(Acknowledging this and its associated points were not originally in the sprint)

Thanks, @jhobs and @Jdlrobson . I agree that this is unplanned (that it was ready to pull in doesn't mean it wasn't originally descoped).

Any updates on this task? It's been a week since work started on the task.

Change 316978 had a related patch set uploaded (by Jhobs):
Fix tooltip interactions

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

Sorry about that Baha. The outage I described yesterday went longer than scheduled. I made some more changes to it this morning and have now pushed the patch to hopefully give a bit of time for review before standup. I'll also be uploading a hygiene patch soon.

phuedx added a subscriber: phuedx.Oct 21 2016, 10:58 AM

Per @bmansurov's review of rEPOPd1f0b182b440: Fix tooltip interactions, it looks as if there's a little more work to be done.

The test still fails for me locally.

We should possibly fix this by doing this: https://gerrit.wikimedia.org/r/318277
I'm not sure what benefit we are adding by attempting to simulate mouse events and this test fails on current master but passes after your patch.

What do you say?

I think the issue you have identified is that binding events inside removeTooltips was a bad idea which I agree with.

Change 318277 had a related patch set uploaded (by Jhobs):
Hygiene: Simplify removeTooltip and restoreTooltip qunit test

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

To keep this task up-to-date, @Jdlrobson and I talked on IRC and I agreed that the QUnit test was behaving as a browser test when it shouldn't have been. The hygiene change is a good fix.

Both rEPOP5aa4ccb1dfe2: Hygiene: Simplify removeTooltip and restoreTooltip qunit test and rEPOP1d028476b66e: Fix tooltip interactions should be good to go if someone wants to merge (or find something to -1 about).

We discussed this in standup sync time. We agreed @jhobs would submit a new patch removing the binding of events in the unit test given the unit being tested has now been simplified. I've just hit +2 now.

Change 318277 abandoned by Jhobs:
Hygiene: Simplify removeTooltip and restoreTooltip qunit test

Reason:
Made redundant by https://gerrit.wikimedia.org/r/#/c/316978/8

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

Change 316978 merged by jenkins-bot:
Fix tooltip interactions

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

phuedx reassigned this task from jhobs to bmansurov.Nov 4 2016, 9:55 AM
bmansurov closed this task as Resolved.Nov 4 2016, 8:19 PM
bmansurov removed bmansurov as the assignee of this task.

Tested on https://en.wikipedia.beta.wmflabs.org/wiki/25thjuneChrome using:

  • Chrome 53.0.2785.143 (64-bit) and FireFox 49.0.2 on Mac OS 10.11.6 (15G1004)
  • IE 11 on Windows 7.

Please note that this broke Javascript on Wikidata, throwing the following error:

TypeError: mw.popups.removeTooltips is not a function. (In 'mw.popups.removeTooltips($elements)', 'mw.popups.removeTooltips' is undefined)

After a little debugging, we found that wikidata is serving an old version of the file inside articles. When accessing the module directly, the said function is not present at https://www.wikidata.org/w/load.php?modules=ext.popups.targets.desktopTarget&debug=true&lang=en&only=scripts&skin=vector.

Cause was found, continue on T150401.