Jdlrobson has uploaded a new change for review.

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

Change subject: Multiple hover events should not clear dwellStartTime
......................................................................

Multiple hover events should not clear dwellStartTime

If a user has hovercards disabled, when they right click a link
this will trigger another hover event which will reset dwellStartTime.
This means when a dwelledButAbandoned event fires shortly afterwards
the totalInteractionTime will not be correct.

To remedy this, the calculation of totalInteractionTime is moved into
ext.popups.schemaPopups

A `hover` event triggers the setting of a dwell start time which is then
reset on a dwelledButAbandoned.

Since dwellStartTime is controlled inside a single place, getMassageData
no longer needs to clear it.

Bug: T147846
Change-Id: Ie5917ca86f0d0ab27f4cf507e6dfa2c271433c03
---
M resources/ext.popups.desktop/ext.popups.renderer.article.js
M resources/ext.popups.renderer.desktopRenderer/desktopRenderer.js
M resources/ext.popups.schemaPopups.utils/ext.popups.schemaPopups.utils.js
M resources/ext.popups.schemaPopups/ext.popups.schemaPopups.js
M resources/ext.popups.targets.desktopTarget/desktopTarget.js
5 files changed, 17 insertions(+), 15 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Popups 
refs/changes/81/316481/1

diff --git a/resources/ext.popups.desktop/ext.popups.renderer.article.js 
b/resources/ext.popups.desktop/ext.popups.renderer.article.js
index 12b03c8..69d9749 100644
--- a/resources/ext.popups.desktop/ext.popups.renderer.article.js
+++ b/resources/ext.popups.desktop/ext.popups.renderer.article.js
@@ -70,8 +70,7 @@
                        if ( data.textStatus !== 'abort' ) {
                                mw.track( 'ext.popups.schemaPopups', $.extend( 
logData, {
                                        action: 'error',
-                                       errorState: textStatus,
-                                       totalInteractionTime: Math.round( 
mw.now() - logData.dwellStartTime )
+                                       errorState: textStatus
                                } ) );
                        }
                        deferred.reject();
@@ -553,8 +552,7 @@
                        mw.popups.settings.open( $.extend( {}, logData ) );
 
                        mw.track( 'ext.popups.schemaPopups', $.extend( logData, 
{
-                               action: 'tapped settings cog',
-                               totalInteractionTime: Math.round( mw.now() - 
logData.dwellStartTime )
+                               action: 'tapped settings cog'
                        } ) );
                } );
 
diff --git a/resources/ext.popups.renderer.desktopRenderer/desktopRenderer.js 
b/resources/ext.popups.renderer.desktopRenderer/desktopRenderer.js
index 32a3074..2fae20a 100644
--- a/resources/ext.popups.renderer.desktopRenderer/desktopRenderer.js
+++ b/resources/ext.popups.renderer.desktopRenderer/desktopRenderer.js
@@ -32,8 +32,7 @@
         */
        function logClickAction( event ) {
                mw.track( 'ext.popups.schemaPopups', $.extend( {}, logData, {
-                       action: mw.popups.getAction( event ),
-                       totalInteractionTime: Math.round( mw.now() - 
logData.dwellStartTime )
+                       action: mw.popups.getAction( event )
                } ) );
        }
 
@@ -42,8 +41,7 @@
         */
        function logDismissAction() {
                mw.track( 'ext.popups.schemaPopups', $.extend( {}, logData, {
-                       action: 'dismissed',
-                       totalInteractionTime: Math.round( mw.now() - 
logData.dwellStartTime )
+                       action: 'dismissed'
                } ) );
        }
        /**
@@ -375,8 +373,7 @@
                var $activeLink = getActiveLink();
 
                mw.track( 'ext.popups.schemaPopups', $.extend( {}, logData, {
-                       action: 'dwelledButAbandoned',
-                       totalInteractionTime: Math.round( mw.now() - 
logData.dwellStartTime )
+                       action: 'dwelledButAbandoned'
                } ) );
                // TODO: should `blur` also be here?
                $activeLink.off( 'mouseleave', leaveInactive );
diff --git 
a/resources/ext.popups.schemaPopups.utils/ext.popups.schemaPopups.utils.js 
b/resources/ext.popups.schemaPopups.utils/ext.popups.schemaPopups.utils.js
index 405b308..c61c392 100644
--- a/resources/ext.popups.schemaPopups.utils/ext.popups.schemaPopups.utils.js
+++ b/resources/ext.popups.schemaPopups.utils/ext.popups.schemaPopups.utils.js
@@ -93,7 +93,6 @@
         */
        function getMassagedData( data ) {
                data.previewCountBucket = mw.popups.getPreviewCountBucket();
-               delete data.dwellStartTime;
                // Figure out `namespaceIdHover` from `pageTitleHover`.
                if ( data.pageTitleHover && data.namespaceIdHover === undefined 
) {
                        data.namespaceIdHover = new mw.Title( 
data.pageTitleHover ).getNamespaceId();
diff --git a/resources/ext.popups.schemaPopups/ext.popups.schemaPopups.js 
b/resources/ext.popups.schemaPopups/ext.popups.schemaPopups.js
index f0c5694..06779a3 100644
--- a/resources/ext.popups.schemaPopups/ext.popups.schemaPopups.js
+++ b/resources/ext.popups.schemaPopups/ext.popups.schemaPopups.js
@@ -1,5 +1,5 @@
 ( function ( $, mw ) {
-       var previousLogData,
+       var previousLogData, dwellStartTime,
                /*
                 *Minimum time to log dwelledButAbandoned events. Initially 
considered as
                 * 250ms to avoid accidental hovers being logged. Now logging 
all events to
@@ -22,6 +22,9 @@
 
                // since this method has side effects only restrict to it to 
the things we actually want
                if ( shouldLog ) {
+                       if ( dwellStartTime ) {
+                               data.totalInteractionTime = Math.round( 
mw.now() - dwellStartTime );
+                       }
                        data = mw.popups.schemaPopups.getMassagedData( data );
                }
 
@@ -32,12 +35,19 @@
                                data.linkInteractionToken === 
previousLogData.linkInteractionToken && action !== 'disabled'
                        ) {
                                shouldLog = false;
-                       } else if ( data.dwellStartTime && mw.now() - 
data.dwellStartTime < DWELL_EVENTS_MIN_INTERACTION_TIME ) {
+                       } else if ( data.totalInteractionTime && 
data.totalInteractionTime < DWELL_EVENTS_MIN_INTERACTION_TIME ) {
                                // if the hover was "accidental" we don't log 
it to server (note while set to 0 this should never happen)
                                shouldLog = false;
                        }
                }
 
+               // Keep track of dwell time
+               if ( !dwellStartTime && action === 'hover' ) {
+                       dwellStartTime = mw.now();
+               } else if ( action === 'dwelledButAbandoned' ) {
+                       dwellStartTime = false;
+               }
+
                if ( shouldLog ) {
                        schemaPopups.log( data );
                }
diff --git a/resources/ext.popups.targets.desktopTarget/desktopTarget.js 
b/resources/ext.popups.targets.desktopTarget/desktopTarget.js
index 60f0181..9f8f627 100644
--- a/resources/ext.popups.targets.desktopTarget/desktopTarget.js
+++ b/resources/ext.popups.targets.desktopTarget/desktopTarget.js
@@ -39,7 +39,6 @@
                var $link = $( this ),
                        // Cache the hover start time and link interaction 
token for a later use
                        eventData = {
-                               dwellStartTime: mw.now(),
                                linkInteractionToken: 
mw.popups.getRandomToken(),
                                hovercardsSuppressedByGadget: 
isNavigationPopupsGadgetEnabled()
                        };
@@ -85,7 +84,6 @@
                mw.track( 'ext.popups.schemaPopups', {
                        pageTitleHover: $this.attr( 'title' ),
                        action: action,
-                       totalInteractionTime: Math.round( mw.now() - 
data.dwellStartTime ),
                        linkInteractionToken: data.linkInteractionToken,
                        hovercardsSuppressedByGadget: 
data.hovercardsSuppressedByGadget
                } );

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie5917ca86f0d0ab27f4cf507e6dfa2c271433c03
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Popups
Gerrit-Branch: master
Gerrit-Owner: Jdlrobson <jrob...@wikimedia.org>

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

Reply via email to