Opened 2 years ago

Closed 22 months ago

Last modified 21 months ago

#5813 closed change (fixed)

Make new options page compatible with edge

Reported by: saroyanm Assignee: agiammarchi
Priority: P2 Milestone:
Module: User-Interface Keywords:
Cc: sebastian, greiner, oleksandr Blocked By:
Blocking: Platform: Edge
Ready: yes Confidential: no
Tester: Ross Verified working: yes
Review URL(s):

https://codereview.adblockplus.org/29673576/
https://codereview.adblockplus.org/29693555/

Description

Background

We are introducing with new options page in #5158, but this was only tested on Chrome and Firefox, but not in Edge.

What to change

Make new options page compatible with Edge browser.

Change History (15)

comment:1 Changed 2 years ago by greiner

  • Platform changed from Unknown / Cross platform to Edge

comment:2 Changed 23 months ago by agiammarchi

Hello there. If nobody is working on this, I'd like to take a chance to make the options page work on Edge too.

I've already spotted few inconsistencies due specific webkit/moz CSS selectors, and one component in particular that does not work as expected in Edge only.

Please let me know if there are other things that I should test, or known issues I can have a look at.

Thanks

comment:3 Changed 23 months ago by agiammarchi

  • Owner set to agiammarchi

comment:4 Changed 23 months ago by greiner

  • Cc oleksandr added

@Ollie Any issues you're aware of that would need to be taken care of specifically?

comment:5 Changed 23 months ago by greiner

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

comment:6 Changed 22 months ago by abpbot

A commit referencing this issue has landed:
Issue 5813 - Make new options page compatible with edge

comment:7 Changed 22 months ago by agiammarchi

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

comment:8 Changed 22 months ago by saroyanm

  • Resolution fixed deleted
  • Status changed from closed to reopened

As mentioned in the Codereview, we still need to make DNT compatible with the edge -> https://codereview.adblockplus.org/29673576/diff/29689576/desktop-options.js#newcode1084

comment:9 Changed 22 months ago by agiammarchi

Since the initial review had a LGTM and if I remember correctly got merged, I have created a new review url for the DNT:
https://codereview.adblockplus.org/29693555/

Feel free to review

comment:10 Changed 22 months ago by abpbot

A commit referencing this issue has landed:
Issue 5813 - Check also window.doNotTrack for Edge

comment:11 Changed 22 months ago by agiammarchi

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

comment:12 Changed 22 months ago by saroyanm

  • Review URL(s) modified (diff)

comment:13 Changed 21 months ago by Ross

The latest uploaded Edge development build is from November 2017. I could try testing this from source, but would rather test a build. Possible for someone to push a new devbuild? Thanks!

comment:14 Changed 21 months ago by sebastian

This issue was only about making the HTML/CSS and fronted JavaScript of the new options page compatible with EdgeHTML. However, Adblock Plus for Microsoft Edge is still build from a branch which hasn't been rebased against the main development branch in a while. It probably makes sense to test the new options page on Microsoft Edge, once we move closer to the next release of Adblock Plus for Microsoft Edge. As for the upcoming release of Adblock Plus for Chrome/Opera/Firefox, it is sufficient to make sure that the changes here didn't break anything on these browsers.

comment:15 Changed 21 months ago by Ross

  • Tester changed from Unknown to Ross
  • Verified working set

Done. Hasn't caused any issues as far as I can tell with the options page on Firefox/Chrome/Opera.

ABP 3.0.2.1998
Firefox 51 / 58 / Windows 10
Chrome 49 / 65 / Windows 7
Opera 36 / 51 / Windows 7

Note: See TracTickets for help on using tickets.