Opened 6 years ago

Closed 3 years ago

#390 closed change (rejected)

Integrate parts of Element Hiding Helper functionality into Adblock Plus

Reported by: trev Assignee: saroyanm
Priority: P3 Milestone:
Module: Adblock-Plus-for-Firefox Keywords:
Cc: arthur, mapx Blocked By:
Blocking: Platform: Firefox
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

http://codereview.adblockplus.org/6525035187535872/
http://codereview.adblockplus.org/5201912261509120/

Description

Background

Parts of Element Hiding Helper functionality seem stable enough for integration into the Adblock Plus extension. This was blocked by #5 but that's no longer a problem.

What to change

Element hiding rules composer and Inspector tool integration can be added to Adblock Plus. The custom element selection mechanism would then stay in Element Hiding Helper, once an element is selected it would trigger the composer from Adblock Plus however.

Change History (15)

comment:1 Changed 6 years ago by arthur

  • Cc arthur added

comment:2 Changed 5 years ago by saroyanm

  • Owner set to saroyanm
  • Platform set to Unknown

comment:3 Changed 5 years ago by saroyanm

  • Platform changed from Unknown to Firefox/Firefox Mobile

comment:4 Changed 5 years ago by saroyanm

What you think guys should we use both composers or somehow combine them ?

I mean current composer that we are using to block images and the Element Hiding Helper's composer.

Last edited 5 years ago by saroyanm (previous) (diff)

comment:5 Changed 5 years ago by trev

It should be both composers - they are very different.

comment:6 Changed 5 years ago by saroyanm

I'll keep them separate.

Wladimir, what you think whether the relevant code should be deleted from EHH ?

I mean the InspectorObserver code and all composer relevant code.

comment:7 follow-up: Changed 5 years ago by trev

Yes, once the relevant code is added to Adblock Plus it should be removed from Element Hiding Helper, we don't want to keep duplicate code around. We should release the new Adblock Plus version first and the new Element Hiding Helper version later to make sure that people aren't stuck without this functionality.

comment:8 in reply to: ↑ 7 Changed 5 years ago by saroyanm

Replying to trev:

We should release the new Adblock Plus version first and the new Element Hiding Helper version later to make sure that people aren't stuck without this functionality.

Maybe in that case we can try to release both ABP and EHH together so we can ask users to update EHH also in case they will notice two "EHH composer" buttons in Firefox Inspector tool ?

Last edited 5 years ago by saroyanm (previous) (diff)

comment:9 follow-up: Changed 5 years ago by trev

We cannot release simultaneously - Mozilla has to review extensions before the updates will be distributed and that can take an arbitrary amount of time. Releasing Adblock Plus first is the best we can do, maybe with some code to hide the button created by Element Hiding Helper.

comment:10 Changed 5 years ago by mapx

  • Cc mapx added

comment:11 Changed 5 years ago by saroyanm

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

comment:12 in reply to: ↑ 9 Changed 5 years ago by saroyanm

Replying to trev:

We cannot release simultaneously - Mozilla has to review extensions before the updates will be distributed and that can take an arbitrary amount of time. Releasing Adblock Plus first is the best we can do, maybe with some code to hide the button created by Element Hiding Helper.

Thanks a lot for info, I've created both discussed reviews.

comment:13 Changed 5 years ago by philll

  • Platform changed from Firefox/Firefox Mobile to Firefox

Made Firefox and Firefox mobile available as seperate platforms.

comment:14 Changed 4 years ago by Compuitguy

+1

comment:15 Changed 3 years ago by trev

  • Resolution set to rejected
  • Status changed from reviewing to closed
  • Tester set to Unknown

With the Web Extensions move, this won't happen any more.

Note: See TracTickets for help on using tickets.