Opened 4 years ago

Closed 3 years ago

Last modified 2 years ago

#3705 closed change (fixed)

Don't rely on info.platform when checking for availability of the chrome.* API

Reported by: sebastian Assignee:
Priority: P2 Milestone:
Module: Platform Keywords:
Cc: scottlow Blocked By:
Blocking: Platform: Edge
Ready: yes Confidential: no
Tester: Ross Verified working: yes
Review URL(s):

https://codereview.adblockplus.org/29338481/

Description

Background

With #3704, info.platform will be set to "edge" (instead "chromium") when running on Edge. However, some of the code in Adblock Plus assumes that info.platform == "chromium" indicates the availability of the Chrome extension API. This however, is no longer be true.

What to change

Adapt the checks relying on info.platform in webRequest.js, lib/prefs.js, and lib/notificationHelper.js to check for availability of the chrome namespace instead.

Change History (7)

comment:1 Changed 4 years ago by oleksandr

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:2 Changed 3 years ago by sebastian

  • Cc scottlow added

comment:3 Changed 3 years ago by oleksandr

  • Resolution set to fixed
  • Status changed from reviewing to closed

comment:4 Changed 3 years ago by sebastian

  • Keywords edge removed
  • Platform changed from Unknown / Cross platform to Edge

comment:5 Changed 3 years ago by sebastian

  • Sensitive unset

comment:7 Changed 2 years ago by Ross

  • Tester changed from Unknown to Ross
  • Verified working set

Marking this as verified for the milestone. Notifications don't seem to work in Edge (#5746) but appear on other platforms. Will follow that up later.

ABP 0.9.11.1849
Edge 40 / Windows 10

ABP 1.13.3.1838
Chrome 49 / 61 / Windows 10
Opera 36 / 47 / Windows 10

Note: See TracTickets for help on using tickets.