Opened 2 years ago

Closed 2 years ago

#22554 closed defect (fixed)

Remove Battery API related bit in Torbutton

Reported by: gk Owned by: tbb-team
Priority: Medium Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: tbb-torbutton, TorBrowserTeam201706R
Cc: arthuredelstein Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

In #5293 we removed the Battery API preference (dom.battery.enabled) from our custom prefs file, 000-tor-browser.js. The rationale was that Mozilla made the preference accessible only by non-webcontent (see: https://bugzilla.mozilla.org/show_bug.cgi?id=1313580) and we therefore don't need to set it ourselves anymore.

We missed that Torbutton is dealing with the pref as well. We should remove that bit, too.

Child Tickets

Change History (8)

comment:1 Changed 2 years ago by gk

Keywords: TorBrowserTeam201706R added
Status: newneeds_review

comment:2 in reply to:  1 Changed 2 years ago by cypherpunks

Replying to gk:

See bug_22554 (https://gitweb.torproject.org/user/gk/torbutton.git/commit/?h=bug_22554&id=5f81b55824ff783abba4052c66e7068279c23c7a) in my public Torbutton repo for a patch.

(Maybe, then thirdparty -> firstparty and sharedWorkers -> null in the next function?)

comment:3 Changed 2 years ago by mcs

r=mcs
This patch looks fine. Should we should also remove the battery status text from the security slider UI? It looks like it is mentioned in the torbutton.prefs.resist_fingerprinting_tooltip entity (within the torbutton.dtd files).

Regarding comment:2, there are several other occurrences of privacy.thirdparty.isolate that also should be fixed.

Last edited 2 years ago by mcs (previous) (diff)

comment:4 in reply to:  3 ; Changed 2 years ago by gk

Replying to mcs:

r=mcs
This patch looks fine. Should we should also remove the battery status text from the security slider UI? It looks like it is mentioned in the torbutton.prefs.resist_fingerprinting_tooltip entity (within the torbutton.dtd files).

Good idea, will do.

Regarding comment:2, there are several other occurrences of privacy.thirdparty.isolate that also should be fixed.

Yes. I added that to our "catch-all"-ticket we currently have for clean-up (#19256).

comment:5 in reply to:  4 ; Changed 2 years ago by mcs

Cc: arthuredelstein added

Replying to gk:

Yes. I added that to our "catch-all"-ticket we currently have for clean-up (#19256).

But shouldn't the code be fixed rather than removed? E.g., torbutton_update_thirdparty_prefs() will never be called, but it seems like we want that functionality. But you or Arthur would know better than me.

comment:6 in reply to:  5 Changed 2 years ago by cypherpunks

Replying to mcs:

Replying to gk:

Yes. I added that to our "catch-all"-ticket we currently have for clean-up (#19256).

But shouldn't the code be fixed rather than removed?

That what I wanted to ask in #19256 :)
But sharedWorkers should be removed.

E.g., torbutton_update_thirdparty_prefs() will never be called, but it seems like we want that functionality. But you or Arthur would know better than me.

Try to change third-party cookies settings in Options ;)

Last edited 2 years ago by cypherpunks (previous) (diff)

comment:7 in reply to:  5 Changed 2 years ago by gk

Replying to mcs:

Replying to gk:

Yes. I added that to our "catch-all"-ticket we currently have for clean-up (#19256).

But shouldn't the code be fixed rather than removed? E.g., torbutton_update_thirdparty_prefs() will never be called, but it seems like we want that functionality. But you or Arthur would know better than me.

Good point. I think we should fix that, although the default we ship works. I opened #22560 for that.

comment:8 Changed 2 years ago by gk

Resolution: fixed
Status: needs_reviewclosed

I applied my fix in bug_22554_v2 (https://gitweb.torproject.org/user/gk/torbutton.git/commit/?h=bug_22554_v2&id=f9b5b78f9629c0429db7f5f1e7c4819df3f4435b) to master (commit f28c1252c7a84b034b35d987fdddfc39a6cf45ba).

Note: See TracTickets for help on using tickets.