To be clear, the linkInteractionToken field is marked with required = false in the schema description; it can (and should) be NULL for pageLoaded events (but yeah, not undefined).

Thanks @Tbayer, good to know.

Right now this doesn't have any effects that I can see, but...

Given the deduplication mechanism we still have, we could be blocking logging of other events that don't have a linkInteractionToken (because they all get stored as tokenToSeenMap[ undefined ] and thus the next ones would not logged).

Right now there are no other events without a linkInteractionToken because the settings cog event only gets logged from the popup cog that has an interaction and thus a token.

But for example if we started logging clicks on the footer as *settings cog* events (as I believe we intend to do) without an interaction, they'd be deduplicated based on undefined token and not be logged.

As said, no immediate effects right now, but this is wrong code and needs fixing.

Thanks for the detailed write-up, @Jhernandez!

Here's what I said in the Hangout not 5 minutes ago:

• User loads a page with PP disabled – pageLoaded event is logged with no Link Interaction Token (LIT)
• User clicks on the footer link, opening the settings dialog
• User enables PP – enabled event is logged with no LIT and we see a collision

And here's why I'm wrong: there's no enabled event. I'm not sure why I thought there was.

As I said, I think that this should be fixed but now I think it's Normal or Low priority.

Change 362315 had a related patch set uploaded (by Pmiazga; owner: Pmiazga):
[mediawiki/extensions/Popups@master] Allow events without linkInteractionToken to be logged

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

Change 362315 merged by jenkins-bot:
[mediawiki/extensions/Popups@master] Allow events without linkInteractionToken to be logged

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

@phuedx with this one you can wait and verify the T167365 (the JS part). If JS part works it means this task also works.