Page MenuHomePhabricator

Live Update causes problems with "Navigation popups" gadget
Closed, ResolvedPublic

Description

Navigation Popups is a gadget that works a little like the Page Previews beta feature, except that it gives previews of all types of links, not just page titles. It's very useful with Recent Changes, because, among other things, it lets you preview diffs of changes without having to visit the page in question. It does this by opening a little popup overlay that shows the diff. Navigation Popups is a popular gadget, with almost 67,000 users.

In Live Update mode, the gadget tends to get "stuck," meaning that individual overlays will decide to hang around on the screen. Normally, a popup disappears as soon as you move the cursor off either it or the link that spawned it. But in Live Updates mode, when a popup gets stuck, there's no obvious way to get rid of it. You can scroll away or click elsewhere on the page. The only thing that clears it is a page reload. The overlay is untethered to its link, and even stays on top when you open a menu (as shown in the screenshot below).

This may well be a bug in the gadget itself, technically, rather than in Live Updates. But I'd like to get this fixed, given the gadget's popularity and usefulness to Recent Changes patrolling, and how bothersome it is when it blocks menus and content.

Screenshots of "Stuck" popup in Live Update mode

Screen Shot 2017-08-08 at 5.39.15 PM.png (851×1 px, 346 KB)
Screen Shot 2017-08-08 at 5.36.55 PM.png (775×1 px, 228 KB)

Event Timeline

jmatazzoni renamed this task from Bug: Live Update causes problems with "Navigation popups" gadget to Live Update causes problems with "Navigation popups" gadget.Aug 9 2017, 9:52 PM
jmatazzoni created this task.

Just in case, a look at Page Previews' behavior on RCs would be a good idea. Page Previews has a similar behavior.

Tacsipacsi subscribed.

I think it's a bug in the gadget, as it has the same problem with live preview. (Until fixed, you can work this around with Popups > Reset.)

I am not 100% sure of the reprecussions of this to other products, so I don't want to edit the actual code, but it seems like it's reasonable to expect that any time wikipage.content hook is fired, the popups should reset.

If that's the case, the dynamicContentHandler should also have a pg.purgePopups(); if not on first run, before setting up the popups again?

Since the dynamicContentHandler goes by specific containers, this should just presumably work; close the popups for the old content and recalculate the ones for the new.
@Tacsipacsi do you know if that will be okay or if it will affect anything else?

The problem is that if a link goes away or disappears, the popup doesn't hide, even after it's not moused. I think popups shouldn't be purged on content change, as the user might use them (e.g. watching a diff or user contribs), where it's very inconvenient if they disappear on every update. Instead, the bug should be fixed that popups somehow "forget" to disappear while they are no longer below the cursor.

I think it's a bug in the gadget, as it has the same problem with live preview. (Until fixed, you can work this around with Popups > Reset.)

The problem is indeed in the gadget.

Just in case, a look at Page Previews' behavior on RCs would be a good idea. Page Previews has a similar behavior.

I have checked your great proposal and Page Previews works good with Live Update feature, only sometimes slight hiccups occur, but generally it works well.

My take on hunting bug that causes this, and one way to solve it, is written on gadget's Talk page. There I explain why Navigation popups associated with link don't get their event handlers hooked, and left without ways of closing them (besides page reload and Popups > Reset workaround).

I have proposed attaching hooks to visible popups after live update, and you can check my proposed solution at my User page.

This gadget claims to have support for MediaWiki's live preview, VisualEditor's saves and Echo's flyout. I have checked my solution to problem explained in this ticket's description on RC page, and for the rest I would ask for your help @Etonkovidova.

I can also take a look at this - the proposed patch looks pretty interesting.

@Petar I see that the task in 'Needs review', so I wait till it will be merged to master. FYI, if it's needed I can check some fixes before merging to master (by checking out just specific git branch to my vagrant). For some cases, it's helpful to see whether there are issues before merging to master.

@Petar I see that the task in 'Needs review', so I wait till it will be merged to master. FYI, if it's needed I can check some fixes before merging to master (by checking out just specific git branch to my vagrant). For some cases, it's helpful to see whether there are issues before merging to master.

This task is a little different from usual workflow, as there is no patch to merge into master, but this is rather bug inside Navigation Popups gadget. As a solution, I have proposed some changes to be made to the gadget, which is mentioned in my comment above.

I have tested it for use cases described in this ticket, but what I've asked from you is to perform some further tests. Please look my previous comment, where you can see what changes you need to make to the gadget code, and test the gadget (with my 'patch'). This gadget claims to have support for MediaWiki's live preview, VisualEditor's saves and Echo's flyout and I have tested it only for Live updates on Recent Changes page.

The reason this ticket is moved to Needs Review is that my solution is out there, and we wait for the response of community members. As of time of this writing, there are only a couple of users who said that they will try it out, but no further feedback from them.

thx, @Petar.petkovic.
I've tested RC and Watchlist pages with the code on my common.js. Unfortunately, betalabs does not receive many updates but with the @Catrope help the testing was done.
The issue is still present.

It turned out, the hover-up (the navigation popup) gets stuck at the exact moment when you

  • have 'Live update' enabled
  • and you hover over some links on RC page

If there are no updates, the navigation popups behave as usual (even with enabled 'Live updates'). If you have a navigational popup at the time the RC page loads new updates, the navigation popup gets stuck. In current production (enwiki 1.31.0-wmf.1) with the constant updates, the issue is easily reproducible, but, e.g. in cawiki (1.31.0-wmf.1) it's very difficult to catch the issue.

So, in betalabs when I was hovering over some links on RC page, a update has come - the popup get stuck. The stuck popup goes aways when 'Live updates' is disabled - this is an improvement: in 1.31.0-wmf.1 the stuck popups do not go away after 'Live updates' turned off.

@Etonkovidova We need to make sure we are on the same page. I have tested Navigation Popups gadget with my patch on RC page (not on WL though) before, and after your comment that issue is still present, I have tested it again. It's working for all intended purposes, but I think our views differ about what "stuck" means.

The problem with Navigation Popups before my change is when Live update items get loaded, any opened popup stays opened and there are no ways of closing it. There is only one workaround mentioned here in the comments, by issuing commands Popup > Reset to the popup.

To illustrate my point visually, here are some recorded gifs of Navigation Popups behavior before and after my change.

BeforeAfter
NavPopups-Original.gif (833×1 px, 2 MB)
NavPopups.gif (833×1 px, 2 MB)

The fact that sometimes popup can stay opened even though link below cursor changed due to fast loading Recent Changes (like on enwiki) is because popup has some additional collision space.
There were comments here T172957#3568636 that explore possibilities of closing all popups upon loading new batch of Live update items, but user can actually be using them. Also new popups can be opened from one popup that is already opened, which can be seen on screen record above. If popup gets closed at that point user might be left confused and/or annoyed, which is very likely to happen on fast changing wikis.

@Petar.petkovic re-checked in enwiki (1.31.0-wmf.1) with importScript('User:Petar.petkovic/common.js');

It works perfectly - thank you for your detailed feedback!

The fix still needs to be applied to the gadget. Who can/should do that, @Catrope?

We will definitely hear from the 60K users if we don't get that fix in.

We will definitely hear from the 60K users if we don't get that fix in.

Can't say more.

Checked in enwiki (1.31.0-wmf.1 ) - looks good.