Last modified: 2013-06-24 11:01:56 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T47824, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 45824 - prepareToSendOnTour() is called twice from jquery.stall
prepareToSendOnTour() is called twice from jquery.stall
Status: RESOLVED FIXED
Product: MediaWiki extensions
Classification: Unclassified
GettingStarted (Other open bugs)
master
All All
: Unprioritized normal (vote)
: ---
Assigned To: Nobody - You can work on this!
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-03-07 05:31 UTC by spage
Modified: 2013-06-24 11:01 UTC (History)
6 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments
Firebug stack trace of first call to prepareToSendOnTour() (197.94 KB, image/png)
2013-03-07 05:31 UTC, spage
Details
Firebug stack trace of second call to prepareToSendOnTour() (198.33 KB, image/png)
2013-03-07 05:31 UTC, spage
Details

Description spage 2013-03-07 05:31:02 UTC
Created attachment 11889 [details]
Firebug stack trace of first call to prepareToSendOnTour()

In Chromium and Firefox I find a single click on a task in GettingStarted results in prepareToSendOnTour() being called twice.

I took screenshots of the Firebug stack each time. They seem identical except:
* 1st call is from jquery.stall.js line 52 in the
    if ( typeof e.target.dispatchEvent != 'undefined' ) test
  and dispatch() shows an event with a timeStamp.
* 2nd call is from jquery.stall.js line 68 in the
    if ( evt === undefined || evt.eventPhase <= 2 ) test
   and dispatch() shows timeStamp=0.
Comment 1 spage 2013-03-07 05:31:57 UTC
Created attachment 11890 [details]
Firebug stack trace of second call to prepareToSendOnTour()
Comment 2 Steven Walling 2013-03-07 05:38:46 UTC
Could this have something to do with the fact that there are actually two tours launched from the page (the single step guider and the actual gettingstarted tour)?
Comment 3 Matthew Flaschen 2013-03-07 06:28:25 UTC
And prepareToSendOnTour is actually not supposed to use stall at all (it's only being used in openTask).  But stall is dispatching the whole click event over again, which means other listeners also get it again.

Good detective work.

Ori, correct me if I'm wrong, but IIRC, we're trying to phase out stall.  I may just replace it in openTask with a preventDefault and doing what we want (e.g. URL navigation in an always).
Comment 4 Steven Walling 2013-03-07 06:30:52 UTC
(In reply to comment #3)
> And prepareToSendOnTour is actually not supposed to use stall at all (it's
> only
> being used in openTask).  But stall is dispatching the whole click event over
> again, which means other listeners also get it again.
> 
> Good detective work.
> 
> Ori, correct me if I'm wrong, but IIRC, we're trying to phase out stall.  I
> may
> just replace it in openTask with a preventDefault and doing what we want
> (e.g.
> URL navigation in an always).

Dario may want to chime in here, so adding him.

For reference: we are looking at gettingstarted click data still, but we're actually using gettingstarted page impressions to create cohorts for analysis, so doing whatever you want to do to ensure no more duplicate events or other weirdness is probably fine.
Comment 5 Ori Livneh 2013-03-07 06:34:08 UTC
(In reply to comment #3)
> Ori, correct me if I'm wrong, but IIRC, we're trying to phase out stall.

I've gone as far as saying that it is known to be broken and should not be used, but S loves that thing. Yes, let's get rid of it.

FWIW, as I've noted in bug 4285 (Comment #5, https://bugzilla.wikimedia.org/show_bug.cgi?id=42815#c5 ), $.ajax({ async: false }) should work, despite not having CORS enabled. But it would require quite a bit of testing.
Comment 6 Matthew Flaschen 2013-03-07 06:35:09 UTC
prepareToSendOnTour does two things:

1. Sets the user pref so they don't get the tour every time they go to Special:GettingStarted
2. When that's done (or in the event it fails), actually send them on the tour.  This means:
2a. Set the tour cookie.
2b. Actually change the URL.

So this function actually has nothing to do with analytics.
Comment 7 Steven Walling 2013-03-07 06:41:08 UTC
(In reply to comment #6)
> prepareToSendOnTour does two things:
> 
> 1. Sets the user pref so they don't get the tour every time they go to
> Special:GettingStarted
> 2. When that's done (or in the event it fails), actually send them on the
> tour.
>  This means:
> 2a. Set the tour cookie.
> 2b. Actually change the URL.
> 
> So this function actually has nothing to do with analytics.

OI heard stall and automatically thought we're talking about click tracking in EventLogging. Sorry to be slow. :)
Comment 8 Ori Livneh 2013-06-24 11:01:56 UTC
'prepareToSendOnTour' has been factored out of GettingStarted in change Ia9931733a
.

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links