Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Availability on AMO #23

Closed
rayman89 opened this issue Jul 11, 2018 · 24 comments
Closed

Availability on AMO #23

rayman89 opened this issue Jul 11, 2018 · 24 comments
Labels
Fixed This issue is resolved help wanted Extra attention is needed question Further information is requested

Comments

@rayman89
Copy link

Sorry if this is not the appropriate place to ask this but why is the addon unlisted on AMO?

@LiCybora
Copy link
Owner

Because Mozilla policy of forbidding assignment of innerHTML and custom DOM library. Changing them is not an easy task as many sites are affected and some with Geo Lock or require account. It will be hard for me to ensure my changes would not affect users. Also deprecating the current DOM library might cause 20 times slower for launch.

This product was on AMO before, but removed by Mozilla due to the above reason. In the past day I tried to replace DOM library with jQuery and use alternative method for innerHTML, but I regret this decision after the need of testing affected sites. Some domains itself looks good but may not be the same story when embedded into other sites, such as video streaming service with anti-adblocking. Therefore I decide to keep it unlisted to make my life easier.

But it is welcomed if there are volunteers willing to participate. Just mark the changes and show some screenshot for relevant site to verify. Then I may re-consider for let it up on AMO. However, inform jspenguin2017 first if anyone really want to perform this job, as he is the owner and developer of this product and he knows much more than me.

Reference:
NanoAdblocker/NanoCore#41 (comment)
https://www.reddit.com/r/firefox/comments/8e8re8/why_dont_more_people_use_firefox/e0dtnnl/

@LiCybora LiCybora added the question Further information is requested label Jul 11, 2018
@LiCybora LiCybora changed the title Question Availability on AMO Jul 11, 2018
@LiCybora LiCybora added the help wanted Extra attention is needed label Jul 11, 2018
@rayman89
Copy link
Author

rayman89 commented Jul 11, 2018

Thank you for the explanation. I know nothing about coding so this might sound stupid but would it be possible to put the prohibited content on for example a userscript or something complementary that the user has to add after installing the addon? If that's possible it would be nice so that less experienced users are able to use this addon and get to know of it's existence. For example I thought the addon was dead because it was not on AMO anymore and AAK cont was discontinued.

I'm not sure if you said you need help testing sites. If it does not require coding only testing I might be able to help with that. Let me know if that's the case.

@jspenguin2017
Copy link

jspenguin2017 commented Jul 12, 2018

Anti-adblock defusers like this cannot be implemented as Userscripts, that is why I rewrote 5k+ lines of code twice to make it a WebExtension (first from Reek's AAK to an Userscript, then from the Userscript to an extension). Modern Userscript hosts have really bad race conditions that cause a lot of problems.

For switching libdom to jQuery, you need to rewrite all 5k+ lines of content rules, as libdom is not designed to be compatible with jQuery and is fundamentally different than jQuery. I am unhappy about how jQuery works, it is an insanely bloated library and it's too late for them to change, so good luck on replacing libdom.

@LiCybora
Copy link
Owner

I'm not sure if you said you need help testing sites. If it does not require coding only testing I might be able to help with that. Let me know if that's the case.

No, not as easy as you think. Some sites with restricted access such as geo lock and require account, which increase difficulty for testing. Even not these cases, at least you have to know why the codes not work and what goes wrong, or it help nothing for fixing.

For switching libdom to jQuery, you need to rewrite all 5k+ lines of content rules, as libdom is not designed to be compatible with jQuery and is fundamentally different than jQuery.

That's why I finally give up and keep it unlisted, at least at this moment. Even I can rewrite them, I don't have time to test all those sites and I believe something would goes wrong. Deviating from upstream might also causing performance/issues difference between two browsers, and probably require two different solutions for some sites (as rules are rewritten). At this moment I want to keep a usable Nano Defender/Adblocker on Firefox first. Whether it is up on AMO is relatively low priority.

@Juraj-Masiar
Copy link

Hello @LiCybora ,
I've started a discussion also on Discourse Mozilla:
https://discourse.mozilla.org/t/get-nano-defender-for-firefox-add-on-to-amo/31214

It's a friendly forum for add-on developers with a lot of skilled people.

@jspenguin2017
Copy link

jspenguin2017 commented Aug 26, 2018

I haven’t tested this out in detail but it seems like it should work

No, it doesn't.

image
image


Go check out how jQuery did it. It's not easy, you'll be looking at 100+ lines of code. The file linked here is only 65 lines but it does use quite a few other files.
https://github.com/jquery/jquery/blob/master/src/core/parseHTML.js
image

@jspenguin2017
Copy link

jspenguin2017 commented Aug 26, 2018

Note: The foregoing assumes that html does not contain its own body tag. I assume that’s true in the context of this extension.

That's not a safe assumption.


  while (j>0){
    s.appendChild(htdoc.body.childNodes[0]); 
    j--;
  }

Why not just:

  while (j--)
    s.appendChild(htdoc.body.childNodes[0]); 

@jspenguin2017
Copy link

Also, for Ace, you'll be looking at 20,000+ lines of code. And yes, Ace uses innerHTML.

uBO uses CodeMirror, gorhill somehow managed to patch innerHTML off CodeMirror. In term of performance, Ace has a longer initial load time but CodeMirror is chunky during runtime, which I don't like.

ND doesn't use Ace for now, but Nano does. If you want both extensions to be on AMO, then have fun with that.

@TonyTough

This comment has been minimized.

@jspenguin2017

This comment has been minimized.

@LiCybora

This comment has been minimized.

@LiCybora LiCybora added the to-do The issue is planned and is working on label Jan 31, 2019
@LiCybora
Copy link
Owner

LiCybora commented Jan 31, 2019

Hello guys, I changed my mind recently. I try my best to patch all innerHTML assignment, although it is just a workaround...

Rewriting libdom...well, I changed the namespace in order not to treat as jquery syntax to bypass the auto-reviewer. Actually I cannot find any policy about forbidding self-written library, let's rethink about it if they ban again.

I update my code in another branch, who are interested can review and feedback. If everything looks good, I plan to let it available on AMO in upcoming version.

TL;DR. For those who just want to test the build, you may download the mozilla version from here:
https://addons.mozilla.org/en-US/firefox/addon/nano-defender-firefox/

NOTE: DO NOT INSTALL BOTH VERSION

@LiCybora LiCybora added Workarounded There exist tricks to deal with this issue, but not perfectly resolve and removed to-do The issue is planned and is working on labels Jan 31, 2019
@ScottRFrost
Copy link

ScottRFrost commented Feb 21, 2019

Nano Defender itself doesn't appear to be available in AMO anymore. Are you planning on porting that from upstream as well, or should we just consider this extension to be used with uBlock Origin on Firefox?

EDIT - Sorry, I meant Nano Adblocker is no longer available in AMO

@LiCybora
Copy link
Owner

Nano Defender itself doesn't appear to be available in AMO anymore.

As of the time writing this comment, Nano Defender still on AMO and what do you mean not appear?

used with uBlock Origin on Firefox

I guess you mean Nano Adblocker not appear in AMO right? If yes then yes I have plan in porting, but I need permission granted by upstream and I still need some preparation. You can find my repository and unofficial build here. Before I settle down, use uBO for now, or if you want help debugging for me, download that unofficial build and report issue to me.

@jspenguin2017
Copy link

jspenguin2017 commented Feb 22, 2019

@LiCybora You can publish it when you think it's stable enough. Just make sure you are keeping it up to date.

As for the integration between NA and ND, you can't really implement that before publishing because you'll probably need the extension ID assigned to you when you publish the first build, and chances are it's just 2 lines of code change.

@LiCybora
Copy link
Owner

Done. Now is online: https://addons.mozilla.org/en-US/firefox/addon/nano-adblocker-firefox/

Also ND need to update for integration working. I will edit all the README soon.

To take care old user who install and forget, GitHub host will still keep update for a while, although it is recommend migrating to AMO version ASAP.

@LiCybora LiCybora added Fixed This issue is resolved and removed Workarounded There exist tricks to deal with this issue, but not perfectly resolve labels Feb 22, 2019
@jspenguin2017
Copy link

OK. Let me know when you have the integration sorted out. I'll link it in my README.

@LiCybora
Copy link
Owner

LiCybora commented Feb 22, 2019

Both NA (1.0.0.88) and ND (15.0.0.96) on AMO can now integrated. However, installing one from AMO and one from GitHub will not be integrated and I do not expected user do so.

@ExE-Boss
Copy link

As for the extension ID, is it the same one as from @jspenguin2017’s release from two‑ish years ago, or is it a different extension ID?

@LiCybora
Copy link
Owner

Different ID. I cannot reuse the one from him without login his account. You will need to reconfigure those settings. Luckily, you can backup settings to file and import to new extension and restore easily.

@jspenguin2017
Copy link

jspenguin2017 commented Feb 23, 2019

Alright, when you are ready, let me know which link you consider to be the home page.
Thanks for your contribution.

@LiCybora
Copy link
Owner

This one as homepage https://github.com/LiCybora/NanoCoreFirefox

@jspenguin2017
Copy link

OK. Looks good.

@LiCybora
Copy link
Owner

Since both extensions are online now, this issue will be closed. I really hope this issue never need to reopen... If any issue other than not found on AMO, please feel free to open a new issue.

I will remove the "experimental" state near the end of this month, as nothing is actually experimenting now.

The GitHub host will still receive update for care old user at least for now, but it will likely strict follow upstream and not receive any platform-specified update (such as disable console). Users are encouraged migrating to AMO and remove the GitHub version ASAP (remember backup you Nano Adblocker first if you use it before).

Waterfox users, although is not supported browser, are however discourage moving to AMO due to lack of dynamic content script API (need FF 59+) which can totally stop ND functioning. They should mod based on the GitHub version/Chromium one if they really want to and accept any risks.

Finally, thanks for everyone support and contribute for this Firefox port.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fixed This issue is resolved help wanted Extra attention is needed question Further information is requested
Projects
None yet
Development

No branches or pull requests

7 participants