Page preview does not close upon closing other page
Closed, ResolvedPublic3 Estimated Story Points
Actions

Assigned To

Authored By

	ovasileva
	Feb 21 2017, 10:05 AM

Description

Steps to recreate:
OSX 10.11.6
Firefox: 51.0.1

Open https://en.wikipedia.beta.wmflabs.org/wiki/Dog
Hover over carnivorous (in the first sentences)
Open in new tab clicking (Ctrl+click or CMD+Click on a Mac) the link
Move mouse and go to other new tab, see hovercard disappear
Close new Carnivore tab, when the Dog tab gets re-focused...

Expected

There's no preview

Actual

The carnivore hovercard will re-appear, and won't close until clicking somewhere or hovering the preview

~~Note: not observed in Chrome or Safari browsers~~

Using the ctrl+link method it also happens on the following OS and browsers:

Windows 10 - Edge & IE 11 & Chrome 56
Windows 8.1 - IE11 & Chrome 56

Sign Off AC

Mark T160545: Bring back keyboard support to PagePreviews as resolved.

Seems like this is related to the focus event on the link being re-triggered when the tab is re-focused...

Details

Related Changes in Gerrit:

	Subject	Repo	Branch	Lines +/-
	Show Page Preview on mouseenter and keyup events	mediawiki/extensions/Popups	master	+3 -3

Customize query in gerrit

Related Objects

Mentioned In: T195718: Cannot dismiss page preview with keyboard navigation
T166610: Page previews shouldn't be shown when user switches back to a tab
T139297: Hovercard images overlapping text/card edge in Internet Explorer
T160545: Bring back keyboard support to PagePreviews
Mentioned Here: T160545: Bring back keyboard support to PagePreviews
P5036 Page Previews keyboard interaction discussion from Thursday, 9th March
rEPOP010ea211f48b: Hide Page Preview after opening link in different tab

Event Timeline

ovasileva created this task.Feb 21 2017, 10:05 AM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 21 2017, 10:05 AM

ovasileva triaged this task as High priority.Feb 21 2017, 10:06 AM

ovasileva moved this task from Incoming to QA Tasks / Needs Reproduction on the Web-Team-Backlog-Archived board.

@Nicholas.tsg - could you try to reproduce this one across different os/browsers? I'm not sure if it's OSX specific.

ovasileva updated the task description. (Show Details)Feb 21 2017, 5:16 PM

ovasileva moved this task from QA Tasks / Needs Reproduction to Upcoming on the Web-Team-Backlog-Archived board.

I've managed to reproduce, but differently (same browser and OS):

Open https://en.wikipedia.beta.wmflabs.org/wiki/Dog
Hover over carnivore
Open in new tab clicking (Ctrl+click) the link
Move mouse and go to other new tab, see hovercard disappear
Close new Carnivore tab, when the Dog tab gets re-focused...

Expected

There's no preview

Actual

The carnivore hovercard will re-appear

Will update description.

• Jhernandez updated the task description. (Show Details)Feb 21 2017, 5:23 PM

ovasileva set the point value for this task to 3.Feb 21 2017, 5:43 PM

I've checked with the redux devtools and a type: "focus" event gets triggered when you head back to the Dog tab which is what triggers the popup to show again.

Previously I could not reproduce the bug. Oddly enough, right-clicking the link and selecting open in a new tab from the options does not reproduce the behavior. However, I was able to reproduce it using the ctrl+link method on the following OS and browsers:

Windows 10 - Edge & IE 11 & Chrome 56
Windows 8.1 - IE11 & Chrome 56

Updated description, thanks @ABorbaWMF

@Jhernandez - the behavior you're observing is acceptable. I'm getting something where the preview appears and won't disappear:

• bmansurov updated the task description. (Show Details)Feb 22 2017, 2:19 PM

@ovasileva It looks the same to me. Did you click somewhere at the end to remove focus off the link? Without talking gifs and videos of this nature are confusing 😄

@Jhernandez - yup, clicking gets rid of it :)

ovasileva moved this task from Backlog to 2016-17-Q3-Goal on the Page-Previews board.Feb 27 2017, 5:14 PM

ovasileva edited projects, added Page-Previews (2016-17-Q3-Goal); removed Page-Previews.

ovasileva moved this task from Needs Analysis to To Do on the Reading-Web-Sprint-93-🔍🔍🔍🔍🔍 board.Mar 1 2017, 6:43 PM

ovasileva moved this task from Upcoming to 2016-17 Q3 on the Web-Team-Backlog-Archived board.Mar 2 2017, 4:30 PM

pmiazga moved this task from To Do to Doing on the Reading-Web-Sprint-93-🔍🔍🔍🔍🔍 board.Mar 3 2017, 4:14 PM

pmiazga claimed this task.Mar 3 2017, 4:17 PM

Change 341818 had a related patch set uploaded (by Pmiazga):
[mediawiki/extensions/Popups] WIP: Hide Page Preview after opening link in different tab

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

gerritbot added a project: Patch-For-Review.Mar 8 2017, 5:08 PM

pmiazga moved this task from Doing to Needs Code Review on the Reading-Web-Sprint-93-🔍🔍🔍🔍🔍 board.Mar 8 2017, 6:35 PM

Given fix works only for CMD+Click or Ctrl+Click. There is no way to detect "Open in new tab" action from context menu nor "middle mouse button click".

• Jhernandez moved this task from Needs Code Review to Needs More Work on the Reading-Web-Sprint-93-🔍🔍🔍🔍🔍 board.Mar 9 2017, 10:37 AM

^ rEPOP010ea211f48b: Hide Page Preview after opening link in different tab has a couple of comments from both @Jhernandez and myself around tidying up the state resetting part of the preview reducer.

Also, @ovasileva, @Jhernandez, and I discussed removing the keyboard interactions (as they are) from Page Previews entirely as we seem to be fighting the UA here. Below is a log of the conversation, which covers the initial idea of removing the keyboard interactions (focus/blur event listeners). @Jhernandez also proposed a possible refinement to the keyboard interaction: show the preview only after the user presses a key /cc @Nirzar.

We should discuss this during today's water cooler meeting.

P5036 Page Previews keyboard interaction discussion from Thursday, 9th March

1	11:03:43 <phuedx> i really don't like this whole "blur on click" thing
2	11:04:27 <phuedx> but it'd require more state to mark the interaction as, say finalised but still open
3	11:06:17 <joakino> it is werid and hacky, yes
4	11:06:59 <joakino> i mentioned to piotr that if we don't find a proper way to do it I'd rather say that we can't do it without removing the focus,blur features of hovercards.
5	11:07:05 <joakino> because of implicit browser behaviors
6	11:09:42 <phuedx> ^
7	11:10:03 <phuedx> that's an interesting tradeoff
8	11:19:27 <phuedx> olliv: would that be acceptable?
9	11:19:39 <phuedx> basically, drop keyboard navigation support
10	11:20:13 <phuedx> which would also get rid of this awkward "open in new tab, tab out, tab in, preview reappears"
11	11:20:56 <olliv> phuedx: I'm not entirely following, keyboard navigation won't work only for the hovercards or for the links as well?
12	11:21:14 <phuedx> only for hovercards
13	11:21:32 <phuedx> sorry, should've been very clear, ONLY FOR HOVERCARDS!
14	11:33:15 <phuedx> olliv: if you tab to a valid link, then it'll show a preview
15	11:33:31 <phuedx> essentially, this is the behaviour that triggers the reappearing preview
16	11:34:34 <phuedx> when you tab back to the page the browser says "you've tabbed to this link!"
17	11:35:41 <olliv> phuedx: that's perfect
18	11:35:47 <olliv> let's do it
19	11:36:52 <phuedx> to be clear, let's remove the tabbing stuff from hovercards?
20	11:40:46 <phuedx> olliv: so, yeah, remove keyboard-related stuff from page previews, i.e. only mouse interactions
21	11:41:18 <olliv> phuedx: yes.
22	11:41:40 <olliv> and I'll make a note to add that to the documentation
23	11:53:05 <phuedx> olliv: do you mind if i copypasta this ^^^ and add it to the task
24	11:53:12 <phuedx> without me swearing, obviously ;)
25	11:53:40 <olliv> phuedx: sure. Could you also tag me with a reminder to update the documentation?
26	11:54:20 <phuedx> sure
27	11:54:25 <phuedx> (that's also on anyone btw)
28	11:58:07 <phuedx> olliv: otoh limiting it to mouse interactions does make it less accessible, some of our users don't use their mouse
29	11:58:10 <phuedx> i dunno
30	11:58:14 <phuedx> it's a balancing act
31	11:58:43 <phuedx> joakino: any thoughts?
32	11:59:13 <phuedx> i'll think about this over lunch
33	11:59:22 phuedx → phuedx\|nom
34	11:59:43 <olliv> if it's only for hovercards, I think it's okay. While I would love for all features to be accessible, I think we can have this as a special case
35	12:01:31 <joakino> olliv: phuedx\|nom: the problem is focus on links, not hovercards, it'd be removing the focus/blur behavior or changing it to require a key press to show the hovercard (like space or enter)
36	12:01:38 <joakino> i'm out for lunch too, finishing a task
37	12:08:10 <olliv> joakino: meaning a user could not focus a link to open via the keyboard, or only that they would not be able to open the hovercard? Also, would this alternative be possible - have a key to show the hovercard and then select the card to open the page?
38	12:11:00 <joakino> olliv: they would be able to focus links, but instead of focus showing the hovercard, having a key (like enter) when focused show the hovercard, and next key (like enter) would open the page
39	12:11:08 <joakino> we can chat later about it
40	12:11:18 <joakino> kind of confusing to do in chat
41	12:11:33 <olliv> joakino: sounds good
42	12:11:48 <olliv> joakino, phuedx\|nom ping me when you're back, maybe we can do a quick hangout
43	12:13:07 <joakino> olliv: the thing is that removing focus on the link to avoid that unexpected behavior makes the focus be lost so if you come back and want to continue tabbing then you'll go to the start of the page
44	12:13:20 <joakino> so fixing that bug means breaking the tabbing experience
45	12:13:58 <olliv> joakino: hmm...okay, yeah, let's talk about it on a hangout and think through the options
46	12:56:16 phuedx\|nom → phuedx
47	12:56:21 <phuedx> olliv, joakino: back
48	13:01:21 <phuedx> right
49	13:01:34 <phuedx> i'm going to grab that chat and remove my swears and whack it on the task for context
50	13:01:42 <phuedx> but we should hangout about this, i agree

pmiazga moved this task from Needs More Work to Needs Code Review on the Reading-Web-Sprint-93-🔍🔍🔍🔍🔍 board.Mar 10 2017, 3:52 PM

I was going to merge this but I think it would be good to document this with adr. Anyone disagree?

In T158631#3099595, @Jdlrobson wrote:

I was going to merge this but I think it would be good to document this with adr. Anyone disagree?

ADRs are for complex architectural decisions. This feels like a product decision. @ovasileva has asked to be reminded to write up this decision on MediaWiki – that's where the on-wiki documentation lives, right?

pmiazga mentioned this in T160545: Bring back keyboard support to PagePreviews.Mar 15 2017, 5:00 PM

pmiazga added a project: Reading-Web-Sprint-94.Mar 15 2017, 5:13 PM

pmiazga moved this task from Needs Analysis to Needs More Work on the Reading-Web-Sprint-94 board.

We discussed this post kick off and a suggestion was made to document inside docs but not inside adr.

As I sat down to write my thoughts about this task, it occurred to me that having T160545: Bring back keyboard support to PagePreviews in the Sprint +1 column in the backlog is a little like punting this task, right?

@phuedx - for this sprint, yes. I'm tagging it with the next sprint, however, so we don't forget.

Huh? This is a little confusing. Why don't we just do it right the first time? Can we really not wait 2 weeks to fix this particular bug that it's worth a temporary disruption to keyboard support...?

I think the concern is that re-adding focus support would be too difficult, could create bit too much complex for such easy feature.

@Jdlrobson @phuedx @Jhernandez - The whole problem is caused by UA triggers focus event on last focused element when a user navigates through tabs. Another idea is to listen to keyup event, it works, everytime you press tab to navigate to another link it triggers the key.... events on specific link. Then we would listen to blur event to dismiss the PagePreview. It will re-add keyboard support but I'm not sure if screen-readers use keyboard events.

phuedx moved this task from Needs More Work to Needs Code Review on the Reading-Web-Sprint-94 board.Mar 16 2017, 5:20 PM

In T158631#3106845, @Jdlrobson wrote:

Huh? This is a little confusing. Why don't we just do it right the first time? Can we really not wait 2 weeks to fix this particular bug that it's worth a temporary disruption to keyboard support...?

Summary of the brief discussion the Reading Web team had around this:

It's not a given that we'll work on T160545: Bring back keyboard support to PagePreviews during the next sprint. We'd rather not release Page Previews to users on the stage0 wikis with this bug present. Given that we're releasing it to those wikis on Monday, 20th, let's drop keyboard support now and commit to implementing it properly later.

@pmiazga may have found a fix that addresses the bug AND keeps keyboard support in tact.

^ Per T158631#3107130.

pmiazga moved this task from Doing to Needs Code Review on the Reading-Web-Sprint-94 board.Mar 16 2017, 8:17 PM

Should be fixed now. Page Previews are triggered on:

mousenter for mouse/pointer interactions
keyup for keyboard navigation

Script listens to the keyup event because a user can press and hold TAB key to quickly jump over the links, we don't want to send multiple AJAX requests for each link. The best solution was to listen to the moment when TAB button is released.

Jdlrobson moved this task from Needs Code Review to Needs QA on the Reading-Web-Sprint-94 board.Mar 16 2017, 8:54 PM

Change 341818 merged by jenkins-bot:
[mediawiki/extensions/Popups] Show Page Preview on mouseenter and keyup events

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

ReleaseTaggerBot added a project: MW-1.29-release (WMF-deploy-2017-03-21_(1.29.0-wmf.17)).Mar 16 2017, 9:00 PM

In T158631#3107834, @pmiazga wrote:

Script listens to the keyup event because a user can press and hold TAB key to quickly jump over the links, we don't want to send multiple AJAX requests for each link. The best solution was to listen to the moment when TAB button is released.

This would make an excellent inline comment 😉

phuedx updated the task description. (Show Details)Mar 17 2017, 4:37 AM

Just checked on https://en.wikipedia.beta.wmflabs.org/wiki/Dog in Chrome - looks good. Over to you @ABorbaWMF

Jdlrobson removed a project: Patch-For-Review.Mar 17 2017, 5:03 PM

From testing on various OS and browsers specified, this seems to be fixed - the hovercard doesn't get stuck on screen when going between tabs. Here are screencaps to show what specifically happens on each -
Windows 8.1 Chrome 57

Windows 8.1 Part 1 Chrome 57.png (1×2 px, 703 KB)

Windows 8.1 Part 2 Chrome 57.png (1×2 px, 698 KB)

Windows 8.1 Part 3 Chrome 57.png (1×2 px, 544 KB)

Windows 8.1 IE 11

Windows 8.1 Part 1 IE11.png (1×2 px, 643 KB)

Windows 8.1 Part 3 IE11.png (1×2 px, 461 KB)

Windows 8.1 Part 2 IE11.png (1×2 px, 698 KB)

Windows 10 Chrome 57

Windows 10 Part 1 Chrome 57.png (768×1 px, 494 KB)

Windows 10 Part 2 Chrome 57.png (768×1 px, 353 KB)

Windows 10 Part 3 Chrome 57.png (768×1 px, 329 KB)

Windows 10 Edge

Windows 10 Part 1 Edge.png (768×1 px, 370 KB)

Windows 10 Part 2 Edge.png (768×1 px, 333 KB)

Windows 10 Part 3 Edge.png (768×1 px, 326 KB)

Windows 10 IE 11

Windows 10 Part 1 IE11.png (768×1 px, 489 KB)

Windows 10 Part 2 IE11.png (768×1 px, 345 KB)

Windows 10 Part 3 IE11.png (768×1 px, 275 KB)

While I think this is fixed I'll allow @ABorbaWMF to decide whether to move it to signoff.

Nicholas.tsg mentioned this in T139297: Hovercard images overlapping text/card edge in Internet Explorer.Mar 20 2017, 10:27 PM

phuedx updated the task description. (Show Details)Mar 21 2017, 4:30 PM

@Nicholas.tsg Performed the tests here. I took a quick look at a couple system/browsers. This appears to be fixed on the beta site.

phuedx reassigned this task from ABorbaWMF to • bmansurov.Mar 22 2017, 7:31 AM

phuedx reassigned this task from • bmansurov to ovasileva.

phuedx added a subscriber: • bmansurov.

looks good, thanks all!

• Jhernandez mentioned this in T166610: Page previews shouldn't be shown when user switches back to a tab.Jun 15 2017, 10:36 AM

pmiazga mentioned this in T195718: Cannot dismiss page preview with keyboard navigation.May 28 2018, 7:43 PM

	F6785673: Windows 10 Part 2 Chrome 57.png
	Mar 20 2017, 9:53 PM

	F6785608: Windows 8.1 Part 1 Chrome 57.png
	Mar 20 2017, 9:53 PM

	F6785607: Windows 8.1 Part 2 Chrome 57.png
	Mar 20 2017, 9:53 PM

	F6785672: Windows 10 Part 1 Chrome 57.png
	Mar 20 2017, 9:53 PM

Page preview does not close upon closing other pageClosed, ResolvedPublic3 Estimated Story PointsActions

Description

Expected

Actual

Sign Off AC

Details

Related Objects

Event Timeline

Expected

Actual

Page preview does not close upon closing other page
Closed, ResolvedPublic3 Estimated Story Points
Actions