jenkins-bot has submitted this change and it was merged.

Change subject: Regression: correctly log the click action total interaction 
time
......................................................................


Regression: correctly log the click action total interaction time

Per T143051, hovercards was asked to be closed after the user clicks on
the hovercard (or the link that showed it). Closing the popup before
logging the `click` event causes `logData` to be lost. This patch logs
the data first and then clears it.

The regression was introduced in Ifd6f75c2a53d8d7b5ef9fd3f232f85b55eea24c8.

Also do not attempt closing a popup when it may not be open.

Bug: T146934
Change-Id: I02febc83036130bcea0a769114c9126cb481bafe
---
M resources/ext.popups.renderer/desktopRenderer.js
M resources/ext.popups.targets/desktopTarget.js
2 files changed, 4 insertions(+), 7 deletions(-)

Approvals:
  Jdlrobson: Looks good to me, but someone else must approve
  jenkins-bot: Verified
  Phuedx: Looks good to me, approved



diff --git a/resources/ext.popups.renderer/desktopRenderer.js 
b/resources/ext.popups.renderer/desktopRenderer.js
index b896a3f..c7e599a 100644
--- a/resources/ext.popups.renderer/desktopRenderer.js
+++ b/resources/ext.popups.renderer/desktopRenderer.js
@@ -147,8 +147,10 @@
                        linkInteractionToken: linkInteractionToken
                };
 
-               $link.on( 'mouseleave blur', mw.popups.render.leaveInactive )
-                       .off( 'click', logClickAction ).on( 'click', 
logClickAction );
+               $link
+                       .on( 'mouseleave blur', mw.popups.render.leaveInactive )
+                       .off( 'click', mw.popups.render.clickHandler )
+                       .on( 'click', mw.popups.render.clickHandler );
 
                if ( mw.popups.render.cache[ $link.attr( 'href' ) ] ) {
                        openTimer = mw.popups.render.wait( 
mw.popups.render.POPUP_DELAY )
diff --git a/resources/ext.popups.targets/desktopTarget.js 
b/resources/ext.popups.targets/desktopTarget.js
index 1db83ad..ceb4592 100644
--- a/resources/ext.popups.targets/desktopTarget.js
+++ b/resources/ext.popups.targets/desktopTarget.js
@@ -138,11 +138,6 @@
                mw.popups.$content = $content;
                $elements = mw.popups.selectPopupElements();
 
-               // On clicking a link the open popup (if any) should be hidden. 
(T143051)
-               $elements.on( 'click', function () {
-                       mw.popups.render.closePopup();
-               } );
-
                $elements.on( 'mouseenter focus', function ( event ) {
                        var $link = $( this );
 

-- 
To view, visit https://gerrit.wikimedia.org/r/313258
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I02febc83036130bcea0a769114c9126cb481bafe
Gerrit-PatchSet: 4
Gerrit-Project: mediawiki/extensions/Popups
Gerrit-Branch: master
Gerrit-Owner: Bmansurov <bmansu...@wikimedia.org>
Gerrit-Reviewer: Jdlrobson <jrob...@wikimedia.org>
Gerrit-Reviewer: Phuedx <g...@samsmith.io>
Gerrit-Reviewer: jenkins-bot <>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to