nusmods icon indicating copy to clipboard operation
nusmods copied to clipboard

Privacy Opt-Out not clickable

Open john43252345 opened this issue 4 years ago • 6 comments

Describe the bug

Privacy Opt-Out not clickable

Accessed on Firefox 86.0 (64-bit)

john43252345 avatar Mar 01 '21 03:03 john43252345

Hi @john43252345, can I just check: did you have adblock turned on for NUSMods?

chrisgzf avatar Jun 26 '21 10:06 chrisgzf

Thanks for the bug report @john43252345!

Triage Outcome

User Impact: Very low

This bug occurs in the privacy opt-out toggle in the settings page. It happens if adblock is turned on.

Cause of bug

Offending function:withTracker in onToggleTracking in SettingsContainer

Everything in withTracker only runs if matomo (the tool we use for analytics) is initialized. See bootstrapping/matomo.ts.

If adblock is turned on, matomo never initializes, and the opting out of tracking is deferred in a queuedTasks array, never to be run.

Steps to resolve

Difficulty: Low

  1. Don't use withTracker
  2. Write a useMatomo hook to get the matomo object.
  3. In SettingsContainer, check if matomo is initialized using useMatomo.
    • if not initialized: show opt-out by default
    • if initialized: use current behavior
  4. Disable the toggle if matomo is not initialized (so that adblock users can't interact with it)
  5. Refactor onToggleTracking to use matomo from useMatomo.
  6. Can some kind soul rename all instance of "mamoto" to "matomo" 😂 (sorry @ZhangYiJiang but your typos around the codebase kinda irks me slightly 😂)

Tests

(If you know how to write unit tests.) Write unit tests with RTL. Assert behaviour depending on whether the matomo object is present or not.

Testing

Take note that matomo is by default not initialized in the dev build. To properly test this, you might need to comment out the relevant code to ensure initializeMamoto() is run in the dev build in entry/main.tsx. You will also need an adblocker of your choice that allows you to toggle on and off. I personally use uBlock Origin. Up to you.

If anyone wants to take it up, do leave a comment here!

chrisgzf avatar Jun 26 '21 11:06 chrisgzf

Hi! I would like to take this up. Your steps to resolve were very helpful for me :) I am a bit confused over the implementation of the useMatomo hook though. Since all we want is to get the matomo object, would the hook simply be:

//found in matomo.ts file

export function useMatomo() {
//where matomo is defined in the same file, matomo.ts
  return matomo; 
}

Any file that would require this hook would then just import in from matomo.ts, instead of having this hook in the src/views/hooks folder

Thanks!

riccqi avatar Aug 09 '21 07:08 riccqi

Hi @riccqi, do you intend to resolve this issue? If not, I would like to take this :)

yaofeng-wang avatar Oct 16 '21 01:10 yaofeng-wang

Hi @riccqi, do you intend to resolve this issue? If not, I would like to take this :)

I was intending to, but was stuck in the process and haven't gotten a reply from the maintainers. Feel free to take over this issue :)

riccqi avatar Oct 16 '21 03:10 riccqi

Hi, I wish to take this issue. Has anyone resolved it?

skrd-18 avatar Jun 17 '22 01:06 skrd-18