Page MenuHomePhabricator

No popup is shown when quickly hovering the same link twice
Closed, ResolvedPublic5 Story Points

Description

The German-Community-Wishlist team continues to run into an issue that feels more and more like it might be an actual blocker for several tasks we would like to work on, most notably T214693: Investigate why Page and Reference Previews interfere when footnote comes right behind link on English Beta Wikipedia, as well as T218765: Show referencePreviews on click.

Steps to reproduce: When quickly hovering a link to a page, leave the link, and quickly hover it again within the time limit of less than a second, no popup will be shown. We found this is always reproducible.

Chain of events:

  • When hovering the link for the first time, one gets a "link dwell" event, as expected.
  • Let the mouse quickly leave the link. This stops everything and triggers an "abandon start" event.
  • When the mouse re-enters the link quick enough, the next event will be a second "link dwell". This will get a new token. The new token will make the code throw this second "link dwell" away, because it happens to touch the same link as the first "link dwell" and the code acts like the first "link dwell" will still be executed. But it won't.
  • The "abandon start" from before was not finished yet, and did not got stopped. It triggers an "abandon end".
  • No popup will ever appear.

Code responsible for this behavior:

There are probably several ways to fix this issue by changing one of the lines of code listed above.

Patch-For-Review:

QA

Testing can be performed on the beta cluster desktop site using the Vector skin: https://en.wikipedia.beta.wmflabs.org/. The changes this task made affected the way events flow through the system. Most especially how network requests for the preview text are issued, succeed, fail, or are canceled. Network requests are canceled in certain conditions when the user quickly dismisses a preview. The logic is holistic and complex.

There are not many configurations to test: anonymous page previews 1) enabled and 2) disabled, logged in page previews 3) enabled and 4) disabled. The desktop Minerva skin could be tested but I don't think it's likely a good use of time given the coverage provided by exercising the Vector skin. In summary, I think the configurations themselves can be tersely smoke tested. Pretty much just making sure that the configurations still exist and previews can be enabled or disabled as an anonymous or logged in user.

So what should be thoroughly tested? In my opinion, it's most important to explore timing on a single enabled configuration. So far as I know, there should never be a scenario where a user can hover over a link and no page preview is shown with the exception of hovering over a link on page load. Here's what I was thinking:

  1. Enable page previews .
  2. Open the DevTools network tab.
  3. Disable the browser cache. Chromium, at least, won't abort requests coming form the cache.
  4. Hover back and forth quickly over a preview. In Chromium, canceled requests are labeled and appear red. This is a good scenario to test. With this change, a preview should always be shown when ultimately resting on a link. Without this change, it was possible to rest upon the link with no preview showing. This may require several attempts.

This last step where a user can dart in and out of a link to show and dismiss the preview is the timing I think that would be most valuable to explore. When the user ultimately dwells on a link, a preview should always be shown regardless of whether previous requests were canceled or not.

QA Results

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

Change 500702 had a related patch set uploaded (by WMDE-Fisch; owner: WMDE-Fisch):
[mediawiki/extensions/Popups@master] Add browser tests for quickly hovering links

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

Change 500702 had a related patch set uploaded (by WMDE-Fisch; owner: WMDE-Fisch):
[mediawiki/extensions/Popups@master] Add browser tests for quickly hovering links
https://gerrit.wikimedia.org/r/500702

The above adds a browser test that tests what the expected behavior described in this ticket would be.

WMDE-Fisch added a comment.EditedApr 2 2019, 4:29 PM

Change 500702 had a related patch set uploaded (by WMDE-Fisch; owner: WMDE-Fisch):
[mediawiki/extensions/Popups@master] Add browser tests for quickly hovering links
https://gerrit.wikimedia.org/r/500702

The above adds a browser test that tests what the expected behavior described in this ticket would be.

FYI: The https://gerrit.wikimedia.org/r/488098 patch seems to fix the issue, at least the test is fine when I rebase it accordingly.

Edit: I totally linked the wrong patch here first

WMDE-Fisch set the point value for this task to 3.
WMDE-Fisch moved this task from Backlog to Review on the WMDE-QWERTY-Season-Sprint-2019-03-20 board.
Jdlrobson moved this task from Product Owner Backlog to Upcoming on the Readers-Web-Backlog board.EditedApr 4 2019, 4:11 PM

@ovasileva the patch for this looks simple, but I'd like you to confirm this is the desired behaviour and if so schedule some time to reviewing/QAing this. Maybe "upcoming" column is a better place for that to happen?

Friendly ping to @ovasileva for scheduling. WMDE has a patch for this but the comments in the patch indicate it may need additional work.

ovasileva changed the point value for this task from 3 to 5.May 2 2019, 5:22 PM
ovasileva moved this task from Upcoming to Product Owner Backlog on the Readers-Web-Backlog board.

Discussed with @Lea_WMDE. As this is high priority for reference previews deployments, I'm moving this back to upcoming and will discuss pulling into the sprint board next grooming.

pmiazga claimed this task.May 20 2019, 5:10 PM

Will review it today/tomorrow.

pmiazga removed pmiazga as the assignee of this task.Jun 10 2019, 5:09 PM
pmiazga added a subscriber: pmiazga.

Dear Readers-Web-Backlog team, I'm just wondering what the status is on the ticket?
Thanks,
Lea

Hey @Lea_WMDE, sorry, I should be more transparent on the phab ticket. I kept the task in doing for some time as I wanted to work on the issue. Proposed solution caused some problems (If I remember right, system was sending an API request everytime link was hovered). Sadly I couldn't find time for it so I moved it back to "to do", to mark it that I'm not working on it.

Apologies that this one is dragging. In a couple of weeks we will be wrapping up our amc project and will likely be able to give this the attention it deserves.

Jdlrobson lowered the priority of this task from High to Normal.Jul 31 2019, 6:51 PM

@thiemowmde, I've revised your patch (https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/Popups/+/488098/). Please review the latest changes when you can.

@thiemowmde, friendly ping on this task. We're moving this out of our sprint board but please note there is a patch (just a revision of your original submission) ready for your consideration when you have the time: https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/Popups/+/488098/. Thank you!

@Niedzielski sorry, I'm only noticing this now. We will look into the patch shortly!

Change 488098 merged by jenkins-bot:
[mediawiki/extensions/Popups@master] Fix action reducer forgetting *all* duplicate dwell actions

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

Change 500702 merged by jenkins-bot:
[mediawiki/extensions/Popups@master] Add browser tests for quickly hovering links

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

ovasileva added a subscriber: Edtadros.
Edtadros claimed this task.Thu, Aug 29, 1:33 PM
awight moved this task from Backlog to Other on the Reference Previews board.
awight moved this task from Other to Doing on the Reference Previews board.
Edtadros reassigned this task from Edtadros to Jdlrobson.Tue, Sep 3, 10:16 PM

@Jdlrobson I have a few questions on this:

  1. In the second paragraph:

There are not many configurations to test: anonymous page previews 1) enabled and 2) disabled, logged in page previews 3) enabled and 4) disabled.

To me, this indicates 4 permutations, but it later says to smoke test them. Even later it says that only one enabled configuration should be tested thoroughly. The test is so small that a smoke test would basically be the complete test. Do you agree that all 4 permutations should be tested?

  1. In test step #4:

canceled requests are labeled and appear red

I don't see any red in the network tab and I would need to know what the label I'm looking for is if it is relevant.

In terms of 1 it does sound like a complete test is needed.
Let me get back to you about the red in the network tab. I think this assumes you can move the mouse faster than your internet connection, so might be easier to test if you enable throttling on your connection say to 3g prior to hovering.

awight moved this task from Doing to Other on the Reference Previews board.Wed, Sep 4, 11:45 AM
Edtadros reassigned this task from Edtadros to Jdlrobson.Wed, Sep 11, 1:20 AM

@Jdlrobson, how do I enable/disable page previews for an anonymous user?

@Jdlrobson, how do I enable/disable page previews for an anonymous user?

Edtadros reassigned this task from Edtadros to Jdlrobson.Mon, Sep 16, 2:05 PM

Test Result

Status:
OS: macOS Mojave
Browser: Chrome
Device: MBP
Emulated Device: iPhoneX

Test Artifact(s):

QA Steps

Testing can be performed on the beta cluster desktop site using the Vector skin: https://en.wikipedia.beta.wmflabs.org/.

There are not many configurations to test: anonymous page previews 1) enabled and 2) disabled, logged in page previews 3) enabled and 4) disabled. The desktop Minerva skin could be tested but I don't think it's likely a good use of time given the coverage provided by exercising the Vector skin. In summary, I think the configurations themselves can be tersely smoke tested. Pretty much just making sure that the configurations still exist and previews can be enabled or disabled as an anonymous or logged in user.

  1. Enable page previews .
  2. Open the DevTools network tab.
  3. Disable the browser cache. Chromium, at least, won't abort requests coming form the cache.
  4. Hover back and forth quickly over a preview. In Chromium, canceled requests are labeled and appear red. A preview should always be shown when resting on a link.

✅ AC1: Anonymous with page previews enabled

✅ AC2: Anonymous with page previews disabled

✅ AC3: Logged in with page previews enabled

❌ AC4: Logged in with page previews disabled
Page previews still appeared even though they were set to disabled.

Edtadros updated the task description. (Show Details)Mon, Sep 16, 2:06 PM

Let's create a new bug to capture and analyse the issue with AC4 and sign this off.

Jdlrobson closed this task as Resolved.Mon, Sep 16, 5:18 PM

Opened T233037 and signed this one off. Thanks for catching that @Edtadros