Opened on 04/27/2014 at 07:19:41 PM
Closed on 03/15/2017 at 03:15:25 PM
#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/ |
||
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.
Attachments (0)
Change History (15)
comment:1 Changed on 04/28/2014 at 02:50:43 PM by arthur
- Cc arthur added
comment:2 Changed on 07/15/2014 at 05:40:57 PM by saroyanm
- Owner set to saroyanm
- Platform set to Unknown
comment:3 Changed on 07/15/2014 at 05:41:17 PM by saroyanm
- Platform changed from Unknown to Firefox/Firefox Mobile
comment:4 Changed on 07/16/2014 at 03:44:46 PM by saroyanm
comment:5 Changed on 07/17/2014 at 06:51:56 AM by trev
It should be both composers - they are very different.
comment:6 Changed on 07/17/2014 at 06:54:50 PM 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: ↓ 8 Changed on 07/17/2014 at 07:10:29 PM 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 on 07/17/2014 at 07:23:46 PM 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 ?
comment:9 follow-up: ↓ 12 Changed on 07/18/2014 at 05:31:41 AM 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 on 07/18/2014 at 11:22:33 AM by mapx
- Cc mapx added
comment:11 Changed on 07/18/2014 at 01:16:14 PM by saroyanm
- Review URL(s) modified (diff)
- Status changed from new to reviewing
comment:12 in reply to: ↑ 9 Changed on 07/18/2014 at 01:17:56 PM 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 on 05/20/2015 at 02:22:39 PM by philll
- Platform changed from Firefox/Firefox Mobile to Firefox
Made Firefox and Firefox mobile available as seperate platforms.
comment:14 Changed on 12/19/2015 at 02:31:44 PM by Compuitguy
+1
comment:15 Changed on 03/15/2017 at 03:15:25 PM 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.
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.