Opened 8 months ago

Last modified 8 months ago

#5750 reopened change

Remove workarounds required for legacy Firefox support

Reported by: kzar Assignee:
Priority: P3 Milestone:
Module: User-Interface Keywords:
Cc: greiner, sebastian, tlucas Blocked By:
Blocking: Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29552642/
https://codereview.adblockplus.org/29555744/

Description

Background

adblockplusui/messageResponder.js conditionally requires some modules. This is so that modules for the Chrome extension can be required without breaking Firefox compatability.

These conditional requires lead to webpack throwing "Critical dependency" warnings and I believe leave to potentially unnecessary code being bundled. Legacy Firefox compatibility is no longer required with the move to web extensions so we should probably remove them.

What to change

  • Make all the module requires in messageResponder.js unconditional, avoiding the "Critical depdendency" warnings. (Something like this.)
  • Remove any other workarounds which are only required for the legacy Firefox extension.

Change History (11)

comment:1 Changed 8 months ago by greiner

  • Priority changed from Unknown to P3
  • Ready set
  • Summary changed from Remove option requires required for Firefox support from messageResponder.js to Remove optional requires required for Firefox support from messageResponder.js

comment:2 follow-up: Changed 8 months ago by kzar

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

Any idea if we can remove the line commented as a Firefox workaround in skin/mobile-options.css?

comment:3 in reply to: ↑ 2 Changed 8 months ago by greiner

Replying to kzar:

Any idea if we can remove the line commented as a Firefox workaround in skin/mobile-options.css?

AFAIK there's nothing related to the legacy Firefox extension in the code for either the mobile or the new options page because those were created with only web extensions in mind.

comment:4 Changed 8 months ago by kzar

Cool OK, so I think the only other question is the Firefox workarounds in adblockplusui/common.js for the openSharePopup function.

comment:5 Changed 8 months ago by kzar

  • Summary changed from Remove optional requires required for Firefox support from messageResponder.js to Remove workarounds required for legacy Firefox support

comment:6 Changed 8 months ago by abpbot

comment:7 Changed 8 months ago by kzar

  • Status changed from reviewing to reopened

comment:8 Changed 8 months ago by sebastian

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

comment:9 Changed 8 months ago by abpbot

comment:10 Changed 8 months ago by kzar

  • Status changed from reviewing to reopened

comment:11 Changed 8 months ago by kzar

  • Blocking 5080 removed
Note: See TracTickets for help on using tickets.