Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#5315 closed change (fixed)

Add support for Microsoft Edge

Reported by: oleksandr Assignee: oleksandr
Priority: P2 Milestone: Adblock-Plus-1.13.4-for-Chrome-Opera
Module: Platform Keywords:
Cc: kzar, sebastian Blocked By:
Blocking: Platform: Edge
Ready: yes Confidential: no
Tester: Ross Verified working: yes
Review URL(s):

https://codereview.adblockplus.org/29458601/

Description (last modified by oleksandr)

Background

We have a version of ABP for Microsoft Edge under the edge bookmark currently. With Windows 10 Creators Update, the amount of Edge specific changes is quite minimal, so we can drop the bookmark and just operate from one version of platform code.

What to change

Move the necessary changes from the edge bookmark to master.


Ticket Summary Owner Type Status Priority Milestone
#3695 Edge exposes it's API with the "browser" namespace instead "chrome" oleksandr change closed P2
#3697 Fall back to i18n.getUILanguage if @ui_locale isn't supported oleksandr change closed P2 Adblock-Plus-1.13.4-for-Chrome-Opera
#3705 Don't rely on info.platform when checking for availability of the chrome.* API change closed P2
#4020 In Edge, rawSender.url is not always defined defect closed Unknown
#4616 Push Edge related changes to the edge bookmark of adblockpluschrome oleksandr change closed P3
#4698 Text not shown in popup window on Edge defect closed P2
#4875 Change app_id paremeter names for Microsoft Edge builds oleksandr change closed Unknown
#5301 Edge prepends extension path to URL when using tabs.create oleksandr defect closed P2
#5321 Workaround Edge not supporting webRequest.ResourceType oleksandr change closed P2
#5582 Workaround Edge bug in tabs.query oleksandr change closed P2 Adblock-Plus-1.13.4-for-Chrome-Opera


Hints for testers

The changes for this ticket touched the following code paths, so it would be preferable to them:

  • We are overriding the core namespace for extensions API. So please make sure that basic functionality like popup page population, localization of options page works.
  • We have changed the way detection of default language works. So please make sure the correct subscription is selected for a language, options page is in correct language, and that left-to-right and right-to-left languages are working fine.
  • We have changed how options page is being opened. Please make sure both options page and first run page can be opened as expected.
  • We have touched the code that deals with our WebSocket wrapper, so checking if WebSocket blocking works would be great. Also if OBJECT and OBJECT_SUBREQUEST filter types work fine.

Change History (14)

comment:1 Changed 2 years ago by oleksandr

  • Description modified (diff)

comment:2 Changed 2 years ago by kzar

  • Cc kzar sebastian added
  • Priority changed from Unknown to P2
  • Ready set
  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:3 Changed 2 years ago by abpbot

A commit referencing this issue has landed:
Issue 5315 - Add support for Microsoft Edge

comment:4 Changed 2 years ago by kzar

So most fixes have made it into master with the above commit. Ollie's now looking into the storage workaround, when he's done we'll either rebase the IndexDB commit or come up with a different workaround and push that to the edge bookmark.

comment:5 Changed 2 years ago by oleksandr

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

The IndexedDB change is described in #5562. Closing this ticket.

comment:6 Changed 2 years ago by kzar

  • Resolution fixed deleted
  • Status changed from closed to reopened

I think this issue should remain open until the edge branch is no longer necessary at all. We're nearly there, but not quite.

comment:7 Changed 2 years ago by sebastian

Also we still need to land #3697 and #5582 in order to complete support for Microsoft Edge.

comment:8 Changed 2 years ago by kzar

  • Description modified (diff)

comment:9 Changed 2 years ago by sebastian

  • Milestone set to Adblock-Plus-for-Chrome-Opera-next

comment:10 Changed 2 years ago by sebastian

I suppose this issue can be closed now? But how about adding a "Hints for testers" section, highlighting the changed code paths which need to be tested on Chrome/Opera as well, in order to make sure that we didn't introduce any bugs there?

comment:11 Changed 2 years ago by kzar

Well don't we still need the edge branch for the storage workaround? IMO we shouldn't close this issue until that's no longer the case.

comment:12 Changed 2 years ago by sebastian

Well, if this issue is supposed to be a meta issue (IMO we don't need one) for everything Microsoft Edge until we abandon the edge bookmark, then there has to be a separate issue for the changes merged upstream in https://codereview.adblockplus.org/29458601/, so that this code gets tested.

comment:13 Changed 2 years ago by oleksandr

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

comment:14 Changed 2 years ago by Ross

  • Tester changed from Unknown to Ross
  • Verified working set

Child tickets verified. Localization, right-to-left text, options page opening, websocket/object/subrequest work as expected.

ABP 0.9.11.1849
Edge 40 / Windows 10

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

ABP 2.99.0.1838beta
Firefox 53 / 55 / Windows 10

Note: See TracTickets for help on using tickets.