Opened 9 months ago

Closed 4 months ago

Last modified 7 weeks ago

#7168 closed change (fixed)

Introduce query string parameter "firstVersion" in notification.json downloads

Reported by: sporz Assignee: mjethani
Priority: P2 Milestone:
Module: Core Keywords:
Cc: fhd, sebastian, kzar, mjethani, wspee, greiner Blocked By:
Blocking: Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Ross Verified working: yes
Review URL(s):

https://gitlab.com/eyeo/adblockplus/adblockpluscore/merge_requests/60

Description (last modified by kzar)

Background

We want to gain insight into our users retention behavior and create the ability to do cohort-based analyses.

Initial brainstorming for this was done internally.

Proposal

We recommend to add an additional query string parameter when downloading notification.json:

  • ?firstVersion=YYYY[MM[DD]][-E]
  • The value of firstVersion would be the value of the version field in the first successful download of notification.json within the client.
  • If there is no firstVersion recorded yet, record lastVersion as firstVersion. If it's not a new user (but one that upgraded from a previous version of Adblock Plus), add "-E" to the firstVersion string.
  • For downloads before the first known successful one, please send a single "0" (new users) or "0-E" (for existing users that just upgraded).
  • We recommend sensible truncating when sending firstVersion due to privacy concerns: during the first full 30 days, 8 characters would be sent (the full date), between 31 and 365 days only 6 characters would be sent (year & month only), after that 4 characters (year only).
  • Date calculations should be independent of the client's clock.

What to change

See implementation of the above proposal.

Hints for testers

We need to test two scenarios: (1) upgrading an existing installation of ABP; (2) fresh installation of ABP.

Upgrade

For a current installation of ABP, upgrade to the new version, then sync the filter lists manually and see that any new filters are available and working.

Move your system clock forward by a day or two and sync manually again.

At no time should there be any errors in the background page and options page consoles.

In the first request for https://notification.adblockplus.org/notification.json, the value of the ?firstVersion= query string parameter should be 0-E. On subsequent requests, it should be the same as the initial value of the version field in the response but with an -E appended to it. All of this can be verified by monitoring the data in the Network panel of the background page console.

For example, if the notification server responds with the following payload:

{
  "notifications": [],
  "version": "201905141155"
}

The next request for https://notification.adblockplus.org/notification.json should have the value of the ?firstVersion= query string parameter set to 201905141155-E.

Unfortunately it may be impossible to test the rest of the logic with just the browser extension as it depends on timestamps from the server, but this is covered in the unit tests.

Fresh installation

Uninstall ABP first and then do a fresh installation, then repeat the above.

The value of the ?firstVersion= query string parameter should follow exactly the same logic but without the -E suffix.

Notifications should work

Any notifications sent by the server should work exactly as they did before the change. It would be a good idea to test all aspects of the notifications system.

All platforms

This change is in a very critical area and therefore should be tested on all supported platforms.

Integration notes

Ensure Prefs.notificationdata.firstVersion defaults to "0" instead of undefined.

Change History (59)

comment:1 Changed 9 months ago by sebastian

  • Cc sebastian kzar added; snoak removed

comment:2 Changed 9 months ago by kzar

  • Cc mjethani added

To submit firstVersion we'd have to keep track of it in the extension. What about for users who we didn't yet track it for, what would we submit?

comment:3 Changed 8 months ago by sporz

  • Description modified (diff)

comment:4 Changed 8 months ago by sporz

  • Updated the description to now cover this case.
  • Updated the description to now do even more truncating after 1 year.

comment:5 Changed 8 months ago by sporz

  • Description modified (diff)

comment:6 Changed 8 months ago by sporz

  • Description modified (diff)

comment:7 Changed 8 months ago by sporz

  • Description modified (diff)

comment:8 Changed 8 months ago by sebastian

Either we keep sending 0 forever for users who upgraded from a previous version, or we we pretend that the first filter list download after updating to a version of Adblock Plus that is aware of firstVersion is the first version for those users. Please clarify?

comment:9 Changed 8 months ago by sporz

You're completely right - and either has serious consequences with regards to ambiguity between users that just installed, existing users that upgraded (or upgraded late) and clients that come with a bundled list. I'll need to follow up on this.

Last edited 8 months ago by sporz (previous) (diff)

comment:10 Changed 8 months ago by wspee

  • Cc wspee added

comment:11 follow-up: Changed 8 months ago by sporz

So, I synced with Kirill and we got a couple of ideas and questions:

Goals:

  • A) For making analyses without ambiguities we should clearly differentiate long-term users that just upgraded from new users. [must have]
  • B) We would like to have workable data on all users right away without a delay (like waiting for new users). [nice to have]

Ideas:

  • a) We were thinking about adding a flag like "-E" to the string sent (like 20190101-E) in case of a long-term user that just upgraded.
  • b) Felix suggested that we already might have an "installation date" for all existing users in their clients database. So for existing users, we could try and use this "installation date" (which wouldn't need to be independent of the clients clock) for creating the string to send.
  • c) In order to check the validity of data from existing users that would send "installation date" (if it's based on the clients clock), for new users we could send it separately (truncated in the same way as firstVersion) as it's own query parameter.
  • d) This would be a temporary thing and removed one release later (when (non-)validity is established).

Questions:

  • For A) and a) - If I interpret your statement above correctly, we can already properly identify in the client if a user just installed OR if he just upgraded. Just to clarify: is this correct? We should make sure this also works when there is a filter list being bundled together with the client.
  • With regards to B) and b) - is there an installation date present on all existing clients already that we could use for this purpose? Is it based on the clients clock?
  • With regards to B) and c)+d) - would a temporary query parameter be something you'd be ok with?
Last edited 8 months ago by sporz (previous) (diff)

comment:12 in reply to: ↑ 11 Changed 8 months ago by sebastian

Replying to sporz:

  • For A) and a) - If I interpret your statement above correctly, we can already properly identify in the client if a user just installed OR if he just upgraded. Just to clarify: is this correct? We should make sure this also works when there is a filter list being bundled together with the client.

Yes, as long as we account for that in the implementation.

  • With regards to B) and b) - is there an installation date present on all existing clients already that we could use for this purpose? Is it based on the clients clock?

There is no API that provides you with the installation date of a Web Extension.

  • With regards to B) and c)+d) - would a temporary query parameter be something you'd be ok with?

I'm not sure how temporary this condition is. We will never have 100% acurrate data until every user that had Adblock Plus installed before we release this change either uninstalled or reinstalled Adblock Plus.

Last edited 8 months ago by sebastian (previous) (diff)

comment:13 Changed 8 months ago by sporz

For A) a) great =]

Clarification for B) - I'd say that using "firstVersion" for all new installations is as reliable as we can get. I don't think we need extra validation on that. I wanted to use it to validate OTHER means of getting information, for upgrading users.

For B) b) - for upgrading users, if there is any date in the client we have access to that's close to the installation date I'd be happy to use it - please suggest any. Otherwise we should create a 'delayed' "first version" for those (starting right after the upgrade), but clearly marking those users (using "20190109-E", and for their first download after upgrade when there is no firstVersion recorded yet "0-E").

I'll wait for your reply before upgrading the tickets description accordingly.

comment:14 Changed 8 months ago by sebastian

So it seems the best option we have is: If there is no firstVersion recorded yet, record lastVersion as firstVersion. If it's not a new user (but one that upgraded from a previous version of Adblock Plus), add "-E" to the firstVersion string.

comment:15 Changed 8 months ago by sporz

  • Description modified (diff)

comment:16 Changed 8 months ago by sporz

Exactly, I added your statement to the description.

comment:17 Changed 8 months ago by sporz

  • Description modified (diff)

comment:18 Changed 8 months ago by sebastian

  • Priority changed from Unknown to P3
  • Ready set

comment:19 Changed 8 months ago by greiner

  • Cc greiner added

comment:20 Changed 8 months ago by mjethani

Maybe a silly question, but how do we ensure that the date calculation is independent of the client's clock?

comment:21 follow-up: Changed 8 months ago by sebastian

firstVersion and lastVersion aren't client-side timestamps, they refer to the value that was given in the Version: comment in the filter list.

comment:22 in reply to: ↑ 21 Changed 5 months ago by mjethani

Replying to sebastian:

firstVersion and lastVersion aren't client-side timestamps, they refer to the value that was given in the Version: comment in the filter list.

OK, so in the first 30 days we send 8 characters, but in order to know if we're in the first 30 days we need to compare firstVersion with the system time?

comment:23 in reply to: ↑ description ; follow-up: Changed 5 months ago by mjethani

Replying to sporz:

  • firstVersion would be the version-number of the first successful download of ANY resource (any filterlist or notification) within the client.

How about we rename this to firstDownloadVersion for additional clarity?

comment:24 in reply to: ↑ 23 ; follow-up: Changed 5 months ago by fhd

Replying to mjethani:

Replying to sporz:

How about we rename this to firstDownloadVersion for additional clarity?

I don't really care what it's called, but to me firstVersion is more logical, because we also have lastVersion and not lastDownloadVersion.

Replying to mjethani:

OK, so in the first 30 days we send 8 characters, but in order to know if we're in the first 30 days we need to compare firstVersion with the system time?

True. I'd suggest we just check the delta between lastVersion and firstVersion - i.e. we assume lastVersion represents the current time. It's not entirely accurate, but it'd keep this simple, and I think it's good enough for what we're trying to do here - reducing entropy.

comment:25 in reply to: ↑ 24 ; follow-up: Changed 5 months ago by mjethani

Replying to fhd:

Replying to mjethani:

Replying to sporz:

How about we rename this to firstDownloadVersion for additional clarity?

I don't really care what it's called, but to me firstVersion is more logical, because we also have lastVersion and not lastDownloadVersion.

lastVersion refers to the last downloaded version of the given resource; firstDownloadVersion would refer to the first downloaded version of any resource (not necessarily the one being requested). Anyway, just a thought.

Replying to mjethani:

OK, so in the first 30 days we send 8 characters, but in order to know if we're in the first 30 days we need to compare firstVersion with the system time?

True. I'd suggest we just check the delta between lastVersion and firstVersion - i.e. we assume lastVersion represents the current time. It's not entirely accurate, but it'd keep this simple, and I think it's good enough for what we're trying to do here - reducing entropy.

Acknowledged.

comment:26 in reply to: ↑ 25 Changed 5 months ago by mjethani

Replying to mjethani:

lastVersion refers to the last downloaded version of the given resource; firstDownloadVersion would refer to the first downloaded version of any resource (not necessarily the one being requested). Anyway, just a thought.

To clarify, if the distinction does not matter to @sporz then it's all the same to me, i.e. I don't really care either.

comment:27 Changed 5 months ago by mjethani

@sebastian @fhd

Here's how I think this should be implemented in Core:

  • Downloadable has a new firstVersion property of type string (null by default); if the value is non-null, it is included as-is in the query string
  • In both lib/synchronizer.js and lib/notification.js:
    • Downloadable.firstVersion is set to the value of Prefs.first_version as-is unless it is undefined (the platform does not support this feature)
    • When a download is successful, if Prefs.first_version is not undefined and parses to a zero-ish value (i.e. covering both "0" and "0-E"), it is updated to the value of the version: field in the response data

All the rest of the logic happens in the platform-specific implementation of the Prefs module, i.e. the getter and setter of the first_version property should take care of stripping out additional characters for privacy, appending a -E as necessary, etc.

@sebastian what do you think?

comment:28 Changed 5 months ago by sebastian

Well, firstVersion is meant to be per filter subscription (analogous to lastVersion), so I don't think we should store any state into a global preference.

comment:29 Changed 5 months ago by mjethani

Wow, then I misunderstood this. So every resource (be it subscription or notification) will have its own firstVersion, is that correct?

(Yes, that's literally what you just said, but it also contradicts what I figured from the current issue description.)

Last edited 5 months ago by mjethani (previous) (diff)

comment:30 Changed 5 months ago by mjethani

  • Ready unset

comment:31 Changed 5 months ago by sebastian

Yes, at least that is what we discussed so far, and also implied if it's supposed to be consistent with the existing lastVersion parameter. At some point we were considering to fall back to some global state when the firstVersion is unknown, but eventually agreed on what I suggested in comment:14.

comment:32 follow-up: Changed 5 months ago by sporz

How about we rename this to firstDownloadVersion for additional clarity?

I agree with Sebastian here, let's keep it consistent and stick to fistVersion.

I'd suggest we just check the delta between lastVersion and firstVersion.

Agreed, it's the only way to be independent of the clients clock. The fact that the information sent will be reduced, e.g. from 8 characters to 6, a bit late in edge cases is not a concern in my view.

firstVersion is meant to be per filter subscription (analogous to lastVersion)

No. fistVersion is meant as a global one, from the first ever successful download of any resource. That's because we'd like to track the clients installation date with it.


That said I got one concern with regards to the delta between lastVersion and firstVersion: This delta needs to be global, too.

Example: If a long-term user adds a new resource, in THIS resource their delta would be low and the information sent would be detailed, while in other resources the information sent would be reduced.

Suggestion: How about we keep two global variables, firstVersion and latestLastVersion. The latter to be used for calculating the delta.

Last edited 5 months ago by sporz (previous) (diff)

comment:33 in reply to: ↑ 32 Changed 5 months ago by fhd

Replying to sporz:

Suggestion: How about we keep two global variables, firstVersion and latestLastVersion. The latter to be used for calculating the delta.

The more I think about it, the more I think we can make our lifes easier, reduce entropy, and still get the information we're interested in if we restrict this to notification downloads. I currently don't see why we'd need firstVersion on filter lists, and we can always add it later if we do (e.g. if we need it for the differential updates in some fashion). That way, notificationdata.firstVersion and notificationdata.lastVersion are the only prefs we need to deal with, _and_ we control the format (third party filter lists could provide arbitrary version formats - I don't think we need to deal with this kind of complexity).

Am I missing something, sporz?

comment:34 follow-up: Changed 5 months ago by sporz

restrict this to notification downloads

I don't fully agree here.

It's correct that we only need this on notification downloads in order to track user retention in a cohort way. What we're interested in mostly is WHY more or less people keep using the product. Investigating this can partly be done by the timing, e.g. with new features or articles being released. However, having the ability to go one step deeper, with cohorts of exceptionrules.txt downloads, easy-privacy.txt, exceptionrules-privacy-friendly.txt, abp-filters-anti-cv.txt or various language-based filter lists makes this tool much more useful for investigating the WHY part.

A second concern is, that we have many products that don't download notification.json at all. Some due to resource constraints on our side, some because they are partner products that won't ever do that (we actually don't have a way of measuring total numbers for those).

If we want to reduce the scope of this project anyway, we should align that with Erez.

Last edited 5 months ago by sporz (previous) (diff)

comment:35 in reply to: ↑ 34 Changed 5 months ago by fhd

Replying to sporz:

It's correct that we only need this on notification downloads in order to track user retention in a cohort way. What we're interested in mostly is WHY more or less people keep using the product. Investigating this can partly be done by the timing, e.g. with new features or articles being released. However, having the ability to go one step deeper, with cohorts of exceptionrules.txt downloads, easy-privacy.txt, exceptionrules-privacy-friendly.txt, abp-filters-anti-cv.txt or various language-based filter lists makes this tool much more useful for investigating the WHY part.

Benefit: firstVersion for exceptionrules.txt and abp-filters-anti-cv.txt should in theory never differ much from the value for notification.json - since they are enabled by default. So what we'd get out of this is: We know _when_ a user started to use a non-default list. I don't know why that'd be crucial to know, but if you, Erez or Daniel have concrete needs for knowing that it's fair enough, no need to explain it here in detail.

Cost: We have to keep a global latestVersion pref, and we'd have to account for filter lists that don't stick to our version format - the easiest way would probably be to just ignore them when it comes to updating latestVersion or sending firstVersion. This approach would make this a little bit more complicated, but not too much, in my book. manish et al can judge that better than me though.

I'd rather get a good enough version of this (e.g. only for notification downloads) into 3.6 than to have a mildly more advanced version in 3.7. We can always make it more advanced later, but right now we're rather in the dark. We could e.g. already start to record firstVersion for all lists, but only send it for notification downloads, for now.

A second concern is, that we have many products that don't download notification.json at all. Some due to resource constraints on our side, some because they are partner products that won't ever do that (we actually don't have a way of measuring total numbers for those).

This one doesn't bother me: Our own products that don't download notification.json because we haven't implemented it yet also won't get these new parameters unless effort is invested. When it comes to partner products: Those that use our SDK should all download notification.json. For those that don't: Fair enough, it's easier to ask them to add these parameters than to ask them to implement notification downloads. But we aren't overly interested in analysing _their_ retention.

comment:36 Changed 5 months ago by erezson

I'd rather get a good enough version of this (e.g. only for notification downloads) into 3.6 than to have a mildly more advanced version in 3.7. We can always make it more advanced later, but right now we're rather in the dark. We could e.g. already start to record firstVersion for all lists, but only send it for notification downloads, for now.

Agree. I always prefer the MVP approach

comment:37 Changed 5 months ago by mjethani

My thoughts on some of the points raised above:

  1. A global lastVersion: If a resource is being requested for the first time, would it be OK to just send firstVersion=0 for it? i.e. if either of firstVersion (global) and lastVersion (resource-specific) is unavailable, we send 0; otherwise it's firstVersion (defaulting to 0) truncated based on lastVersion - firstVersion. In this case, we would not need a global lastVersion.
  1. Name of global firstVersion: If we're going for a global firstVersion and we're not sure if we may need a resource-specific firstVersion at some point (also possibly a global lastVersion), we should just make the names reflect the global status in some way.
  1. Format of version: field's value: For external filter lists, we already assume that the value must parse with JavaScript's parseInt() (base 10). This assumption already exists in the code.
  1. Minimal/partial implementation as suggested by @fhd: Sounds good to me from an implementation perspective. I still need clarity on whether we're going for a global firstVersion or a resource-specific one.

Additionally: I'm wondering why we should send firstVersion= and other parameters to external hosts? We are moving to HTTPS-only downloads (core#5), which is a step in the right direction, but I'm still not sure why external hosts need to know about firstVersion.

comment:38 Changed 5 months ago by sporz

If a resource is being requested for the first time, would it be OK to just send firstVersion=0 for it?

Ideally, the date and time of installation would be sent, for any resource download, including the very first. Knowing when a new resource is downloaded for the first time is very value-able to us in determining differences between users that did or did not stay with us from a specific cohort.

For determining the installation date, using the first ever successful download is a proxy, as we cannot trust the clients clock. Using one firstVersion per filter list is NOT a proxy for that installation date, but rather signifies the date the option was ticked. This removes the most important analysis angle.

Name of global firstVersion

I'm stopping comments on that, and please scratch my last one. You figure it out, we're fine with anything you come up with.

external lists

When the request is made to any host by eyeos, do we need to send ANY query string parameters at all?

Minimal/partial implementation

Erez and Felix are main stakeholders of this project. My feedback is that partial implementation already gives some useful information on big actions we take. There's more to be gained if we extend the approach to the full scope.

Last edited 5 months ago by sporz (previous) (diff)

comment:39 follow-ups: Changed 5 months ago by mjethani

Alright, here's a quick specification of how this will work.

Let's define some terms first:

  • server: The Adblock Plus notification server, i.e. https://notification.adblockplus.org/
  • request: An HTTP request to the server
  • response: An HTTP response from the server
  • timestamp: The value of the Date header in a response
  • client: Any Adblock Plus installation

There will be two values maintained on the client:

  • first_response_date = 0: The timestamp in the very first response from the server
  • last_response_date = 0: The timestamp in the most recent response from the server

For every request to the server, the client will:

  1. Include a ?firstResponse= parameter in the query string with the value set to the YYYYMMDD part of first_response_date
  2. Compute d = last_response_date - first_response_date
  3. If d > 30 days, drop the DD part from the value of ?firstResponse=
  4. If d > 365 days, drop the MM part from the value of ?firstResponse=
  5. If [to be determined (@sebastian?)], append -E to the value of ?firstResponse=
  6. Send the request
  7. If the response is the first ever from the server (i.e. first_response_date == 0), set first_response_date to the timestamp in the response
  8. Set last_response_date to the timestamp in the response

A few things to note:

  1. This is completely independent of any existing query string parameters in requests or any values in the response payloads
  2. In the future, it would be possible to decouple the timestamps from the notification server by adding a little more code to the client, without breaking existing clients (e.g. pinging ping.adblockplus.org separately at regular intervals)
  3. We could include ?firstResponse= in other requests as well (e.g. subscription downloads), but we will need to resolve some questions before we can do this (I won't go into details here, let's file a separate ticket)

Any questions or comments?

Last edited 5 months ago by mjethani (previous) (diff)

comment:40 in reply to: ↑ 39 ; follow-up: Changed 5 months ago by sebastian

Replying to mjethani:

  1. If [to be determined (@sebastian?)], append -E to the value of ?firstResponse=

The idea (not mine) was, to add -E to distinguish request from existing users after updating to a version of the client that is aware of firstVersion from the first request sent for new users.

comment:41 in reply to: ↑ 40 Changed 5 months ago by mjethani

Replying to sebastian:

Replying to mjethani:

  1. If [to be determined (@sebastian?)], append -E to the value of ?firstResponse=

The idea (not mine) was, to add -E to distinguish request from existing users after updating to a version of the client that is aware of firstVersion from the first request sent for new users.

My question is about how core would know about this. Is there a setting/preference in the extension that it can access to determine whether to add -E?

comment:42 in reply to: ↑ 39 ; follow-up: Changed 5 months ago by sporz

Replying to mjethani:

Alright, here's a quick specification of how this will work.

Sounds pretty good to me =]

  • server: The Adblock Plus notification server, i.e. https://notification.adblockplus.org/

This also covers filter list downloads to our servers, e.g., https://easylist-downloads.adblockplus.org, right? I'm concerned as some products will never download notification.json.

comment:43 in reply to: ↑ 42 ; follow-up: Changed 5 months ago by mjethani

Replying to sporz:

Replying to mjethani:

  • server: The Adblock Plus notification server, i.e. https://notification.adblockplus.org/

This also covers filter list downloads to [emphasis mine] our servers, e.g., https://easylist-downloads.adblockplus.org, right?

You mean from our servers, right?

Ideally yes, but:

We could include ?firstResponse= in other requests as well (e.g. subscription downloads), but we will need to resolve some questions before we can do this [emphasis added]

The main question is how to tell if it is our server. Should we go by the hostname pattern (i.e. *.adblockplus.org), an explicit whitelist of hostnames, or something else?

Also, are the clocks between the notification server and the filter list server guaranteed to be in sync?

We could use the timestamps only from the notification server but send the ?firstResponse= parameter to all of our servers (by some definition of "our"), or we could also use the timestamps from any of our servers ... or we could just have a separate timestamp server.

These are some things we need to sort out, but we could start with the current specification above and accept that it may only work for the WebExt version.

comment:44 in reply to: ↑ 43 Changed 5 months ago by sporz

Replying to mjethani:

You mean from our servers, right?

Yes.

The main question is how to tell if it is our server. Should we go by the hostname pattern (i.e. *.adblockplus.org), an explicit whitelist of hostnames, or something else?

From data side I don't really care if we do requests with query string parameters to hosts not controlled by eyeo. I only want to make sure that we DO when it IS one of ours. From a privacy perspective, of course, it might make sense to limit it. How we do that best, I'm not sure.

Some of our partners are using the same mechanism to download their own filter lists (AdBlock, Opera, maybe Microsoft) - I don't know if they're using the parameters for anything serverside. If they do, we might break it.

Also, are the clocks between the notification server and the filter list server guaranteed to be in sync?

Yes, they are (currently) the same physical server (on different domains). I wouldn't worry about it, because these servers are under our control.

We could use the timestamps only from the notification server but send the ?firstResponse= parameter to all of our servers (by some definition of "our"), or we could also use the timestamps from any of our servers ... or we could just have a separate timestamp server.

As stated above, some of the products don't download notification.json. We cannot rely on it as a linchpin in the long run.

These are some things we need to sort out, but we could start with the current specification above and accept that it may only work for the WebExt version.

Yes, starting with WebExt only makes sense to me. Then we need to make sure other products don't send this parameter (if core is updated for them), or if they do, send a clear indication that it's not ready to be used.

Last edited 5 months ago by sporz (previous) (diff)

comment:45 Changed 4 months ago by mjethani

I have a work-in-progress patch now.

After working on this, I have realized that some of the discussion I started above is not really valid for this implementation. Here's how it works:

  1. There's a firstVersion (not firstResponse, see rationale below) value on the client, which is set to "0" by default. If this value is not present, nothing happens, which means clients have to explicitly opt in to this feature.
  2. After the first download, the value of lastVersion (based on the version value in the payload) is assigned also to firstVersion. If there is any notification data on the client, then it is considered to be an existing installation and an -E is appended to this value. e.g. if the server returns "version": "201905071234" in the first download and some notification data already exists on the client, the value of firstVersion is set to "201905071234-E".
  3. For the initial request, the value of the ?firstVersion= query string parameter is "0" or "0-E" depending on the above detection logic.
  4. For subsequent requests, the value of the ?firstVersion= query string parameter is what was set initially (see point 2 above), modulo any privacy-related trimming. e.g. if the difference between lastVersion and firstVersion exceeds 30 days, the value would be in YYYYMM[-E] format.

Some rationale:

  1. Why firstVersion and not firstResponse: This is specific to notifications right now. If we can do this for all resource downloads, then we can work on a different design/semantics (also see challenges below).
  2. Why version from payload rather than Date: header: The fetch API does not give us response headers on all platforms. If we want to support all platforms based on a header value, we'll have to implement some hacks, which goes against what we're trying to accomplish in general (i.e. cleaning up and simplifying things). Since this implementation is specific to notifications anyway, we can instead just go with the original idea of using the version value, which is practically a date anyway.

The scope of this ticket is now narrowed down to this specific implementation of this idea. Let's open a new ticket (now or in the future) to discuss this for all resource downloads. I'm not even sure that it's going to be necessary. (Note: There's a lot happening, e.g. discussions about rewriting Core for other platforms, etc. Let's focus on solving the immediate problem here.)

comment:46 Changed 4 months ago by mjethani

  • Description modified (diff)
  • Priority changed from P3 to P2
  • Ready set
  • Summary changed from Introduce query string parameter "firstVersion" in resource downloads to Introduce query string parameter "firstVersion" in notification.json downloads

comment:47 Changed 4 months ago by mjethani

  • Owner set to mjethani

comment:48 Changed 4 months ago by mjethani

  • Review URL(s) modified (diff)

comment:49 Changed 4 months ago by mjethani

  • Status changed from new to reviewing

comment:50 Changed 4 months ago by mjethani

  • Description modified (diff)
  • Resolution set to fixed
  • Status changed from reviewing to closed

comment:51 Changed 4 months ago by mjethani

I have added some testing hints as best as I could. If testers need any clarifications I am happy to help.

comment:52 follow-up: Changed 7 weeks ago by Ross

Okay, try as I might I cannot get this parameter to appear whatsoever in the requests. The lastVersion query parameter works as expected but the firstVersion parameter is never included.

I've been testing using unpacked devbuilds and attempting to upgrade from 3.5.2 equivalent to latest devbuild (3.5.2.23340). I've also tried various other to/from combinations. I've also tried giving the extension notification.json files with different dates.

I'm not sure if this is not actually working, or I'm missing something?

ABP 3.5.2.2340
Chrome 75.0.3770.142 / Windows 10 1809

comment:53 in reply to: ↑ 52 Changed 7 weeks ago by kzar

Replying to Ross:

Okay, try as I might I cannot get this parameter to appear whatsoever in the requests. The lastVersion query parameter works as expected but the firstVersion parameter is never included.

I'm not sure to be honest Ross! On my system, Prefs.notificationdata.firstVersion is undefined and looking at the code I can see why the firstVersion parameter wouldn't be included in that case. But, I wasn't really involved in this one and the testing instructions seem to say that the parameter is always expected to be included.

Could you clarify Manish?

comment:54 Changed 7 weeks ago by kzar

If I understand it right, we only set Prefs.notificationdata.firstVersion if its value is "0" but it's value will be undefined initially and so it will never get set.

comment:55 Changed 7 weeks ago by mjethani

I wrote in comment:45:

There's a firstVersion (not firstResponse, see rationale below) value on the client, which is set to "0" by default. If this value is not present, nothing happens, which means clients have to explicitly opt in to this feature.

The extension should set the firstVersion value to "0" in lib/prefs.js.

comment:56 Changed 7 weeks ago by kzar

  • Description modified (diff)

OK, thanks that makes sense. I've added the integration notes.

comment:57 Changed 7 weeks ago by kzar

  • Description modified (diff)

comment:59 Changed 7 weeks ago by Ross

  • Tester changed from Unknown to Ross
  • Verified working set

Done. Tested and working as described in Chrome/Opera/Firefox for now for 3.6.1.

I've made a note to check this on the remaining platforms after 3.6.1.

ABP 3.6.0.2353
Chrome 49.0.2623.75 / Windows 10 1809
Chrome 76.0.3809.87 / Windows 10 1809
Opera 62.0.3331.72 / Windows 10 1809
Firefox 51.0 / Windows 10 1809
Firefox 68.0 / Windows 10 1809

Note: See TracTickets for help on using tickets.