Page MenuHomePhabricator

Add reading depth to hovercards schema (defined as time spent on page)
Closed, ResolvedPublic5 Estimated Story Points



  • The existing Popups schema is used to log:
    • The total amount of time the user spent looking at a page (determined by the work already done by @bmansurov)
    • The total amount of time the page was open.
  • One Popups event is logged before the page is unloaded by the UA.
  • No events are logged if the UA doesn't support the beacon API.
  • This work is done in the Page Previews codebase.

Add reading depth to popups schema based on discussion in T145388: [Spike 1hr] Investigate alternate ways of defining reading depth, such that an event is logged at set intervals while a reader explores a page.


Add a new action type of event and field to Schema:Popups to track time on page, with analogous values to the checkin field on Schema:TestSearchSatisfaction2

type "integer"
required false
description "A numeric value representing the number of seconds a user has spent on a page. As with Schema:TestSearchSatisfaction2, the pings are at 10s, 20s, ..., 50s, 60s, 90s, 120s (2min), 150s, 180s (3min), 210s, 240s (4min), 300s (5min), 360s (6min), 420s (7min)."

And also add a variable for the visibility of the page ( visible or hidden for supported browsers, null or undefined for not supported ones.

Use the existing sampling rate for Hovercards.

Change timer based on visibility value. If visibility is "hidden" or "unknown" pause time. Visibility values may not have to be logged.

Note: browsers which do not support visibility will be paused throughout - these will be filtered out in analysis.

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
Jdlrobson updated the task description. (Show Details)Oct 17 2016, 4:21 PM
MBinder_WMF set the point value for this task to 5.
bmansurov moved this task from To Do to Doing on the Reading-Web-Sprint-87-♨️😭 board.

The schema has been updated:

I went with Tilman's suggestion of using the Fibonacci numbers as the other option was also fairly arbitrary and mostly useful for the search schema.

Change 327122 had a related patch set uploaded (by Bmansurov):
WIP: Add reading depth

@bmansurov When you updated the schema, I noticed you added pageVisibility as a property, however, the Discovery team used that API to pause the timer when the page was hidden. Given a hidden page could generate a large number (currently 19) of unusable data points, would it not make more sense to follow Discovery's lead and only log checkin time, using the pageVisibility API to pause the timer? See for Discovery's implementation.

@jhobs I think that's an overkill personally. I'd like to keep it simple. @ovasileva what do you think?

@bmansurov - it depends. Would we be in danger of logging too many data points? What happens if a tab is open for days (months!) in a row?

@ovasileva logging happens at these seconds: 1, 2, 3, 5, 8, 13, 21, 34, 55, 89, 144, 233, 377, 610, 987, 1597, 2584, 4181, and 6765. So after about 100mins logging stops.

@bmansurov - I think we're good as is then.

@bmansurov @ovasileva The problem with this approach though is that if someone opens a tab to read later, then comes back to it after any significant period of time, our depth measurement is wildly skewed. If they leave it for over 100 minutes then we won't have any useful depth measurement at all. I know I personally open several tabs all the time when reading an article in order to read them after I'm done with my current one (especially on mobile). Pausing a timer when not visible ensures we get accurate logs, fewer irrelevant data points, and it's already been done and field-tested by another team, so it's a simple implementation (basically just copy-paste).

@jhobs, we always have misleading data, even when we pause the timer. For example, if you have a tab open and then have another application on top of it, we would not pause the timer because as far as the browser is concerned the tab is visible, but in fact the user is doing something else. That's why it's tricky. Besides, if you're tab is not visible and you're on the browser window looking at a different tab, we'd actually be capturing some useful information - the user has the tab open, but it's not visible. This can actually help us in determining if users leave tabs open and not read or read.

So yes, it's unlikely that not pausing the timer will cause an unmanageable amount of extra events. However, I agree with all of @jhobs's points at T147314#2874180 . While it's true that measuring how often and how long tabs are not visible might at some point yield useful information, it's orthogonal to the stated purpose of this instrumentation, namely to measure user engagement with the content (see also T148262 ). For that, we want to get the best possible estimate for the time the user has actually spent reading the page, and the Discovery team's approach of pausing the timer seems to be more suited for that. It would be less accurate (and presumably also quite a pain during data analysis) if one keeps counting and has to later decide how to deal with checkins sent while invisible.

@ovasileva given T147314#2876217, would you please update the task description so that I implement the desired solution? Thanks.

ovasileva updated the task description. (Show Details)Dec 15 2016, 5:16 PM

@bmansurov - updated description to allow for pausing timer based on visibility

@ovasileva if we pause, we don't have to log the page visibility information, do we?

@ovasileva if we pause, we don't have to log the page visibility information, do we?

We discussed this in standup and have clarified that we only need the "checkin" value with the number of seconds.

ovasileva updated the task description. (Show Details)Dec 15 2016, 6:26 PM

Change 327697 had a related patch set uploaded (by Bmansurov):
WIP: Add reading depth

Change 327697 abandoned by Phuedx:
WIP: Add reading depth

Wrong branch.

I think I need to add one more set of test cases. For now, please review the other parts of

Change 327122 merged by jenkins-bot:
Add reading depth

Change 330389 had a related patch set uploaded (by Phuedx):
Hygiene: Avoid touching internal state in tests

phuedx added a subscriber: phuedx.Jan 4 2017, 11:16 AM

rEPOP60adbd083154: Hygiene: Avoid touching internal state in tests is a minor follow-on change that only touches a test. Moving this to Ready for Signoff per my review of rEPOP4afa1958a054: Add reading depth, which was as follows:

Generally speaking, this LGTM.

I've tested it locally in Chrome (55.0.2883.95), which supports the Page Visibility API, and in IE11 on Windows 10, which doesn't. In Chrome I saw "checkin" events being logged at the appropriate times and saw that they weren't logged if I tabbed away from the page or minimised the browser. In IE11 I saw no such events being logged.

Change 330389 merged by jenkins-bot:
Hygiene: Avoid touching internal state in tests

bmansurov removed bmansurov as the assignee of this task.Jan 4 2017, 6:03 PM
bmansurov added a subscriber: Jdlrobson.

@Jdlrobson can you sign off on it as it's a technical task?

I happened to mention this endeavour to @Krinkle the other day, and he suggested a different method, which seems much simpler and more accurate at the same time:
Instead of sending a ping in predefined intervals, simply use the onbeforeunload event once at the end when the tab is closed. (One limitation of this is that it requires a sendBeacon capable browser, but we do that anyway in this schema.)
This would yield much more accurate estimates of the actual time spent on the page, e.g. in the sense that we don't need to interpolate between Fibonacci numbers or other intervals when calculating average. It would also address the concerns in T155081 (although I'm not sure), this would also limit the number of additional events generated per pageview to 1. And it looks like other people have been using it for the same purpose before, see e.g. (which also handles tab visibility in that setup).
Thoughts? The only downside I could imagine right now concerns cases where the browser process ends abnormally in a way that prevents the onbeforeunload event from being sent (crash, power outage, etc.).

@Tilman, so there will be one check in at the end of the session, right? Another downside happens when the user closes a tab when there is no internet connection. This would prevent the browser from sending the information we need.

@Tilman, so there will be one check in at the end of the session, right?

Yes, that would be the idea.

Another downside happens when the user closes a tab when there is no internet connection. This would prevent the browser from sending the information we need.

How would the current interval solution fare in that situation (no internet connection when an event is sent)?

How would the current interval solution fare in that situation (no internet connection when an event is sent)?

Since we have multiple checkin points, a drop in connection may affect some of the checkins, hopefully not all. With only one checkin point a drop in connection at the time of checkin will result in total loss of data. I'm not sure if that's a good thing or not, come to think about it.

Minimising the number of events is a worthy (and necessary) goal.

As @bmansurov notes, we risk a complete loss of data if the user's connection is poor when the UA tries to make the asynchronous request. AFAIK sendBeacon makes no guarantees of delivery, only that the request will be sent without blocking and that it'll be scheduled before unload. If we were to send multiple beacons while the page is visible, then a few requests may succeed resulting in incomplete data, which we can mostly guard against with SELECT MAX(time) ...

phuedx updated the task description. (Show Details)Jan 18 2017, 6:30 PM

moving this back to signoff, as a way to compare method implemented here with T155639: Create reading depth schema

@bmansurov, @phuedx - since we won't be implementing a new structure, I think we're ready to close this.

ovasileva closed this task as Resolved.Jan 30 2017, 12:14 PM
ovasileva claimed this task.