Skip to content
This repository has been archived by the owner on Dec 23, 2018. It is now read-only.

Add WebExtension port of this add-on #48

Merged
merged 13 commits into from
Apr 17, 2017
Merged

Add WebExtension port of this add-on #48

merged 13 commits into from
Apr 17, 2017

Conversation

ntninja
Copy link
Collaborator

@ntninja ntninja commented Jan 9, 2017

Most stuff was rewritten and (if possible) simplified, the only truely original piece of code is the policy engine that processes the whitelist entries.

Also contains a newly "designed" logo that is used for the toolbar icon requested #27.

Except for not preserving the previous settings this add-on does everything the previous iteration of Smart Referer did and more!

@ntninja
Copy link
Collaborator Author

ntninja commented Jan 9, 2017

For a discussion about how settings migration might work see #47.

@meh
Copy link
Owner

meh commented Jan 9, 2017

Just a note I'm not ignoring all of this, it's just I'm on vacation 🐼

Most stuff was rewritten and (if possible) simplified, the only truely original piece of code is the
policy engine that processes the whitelist entries.
Also features a new "logo" for Smart Referer.
@ntninja ntninja changed the title [WIP/RFC] Add WebExtension port of this add-on Add WebExtension port of this add-on Jan 9, 2017
@ntninja
Copy link
Collaborator Author

ntninja commented Jan 9, 2017

So here is the final version of this (for now):
screenshot_20170109_215630

As you can see I've added a more user-friendly UI for managing exceptions. Additionally smart referer can now be enabled and disabled using the checkbox in the options OR using the icon on the toolbar. Everything else is unchanged (aside from be implemented as a WebExtension).

@meh Thanks for your feedback on your current “status”. Have a nice holiday! 🙂

Known issue: Options page not displayed by this version (but on-off-toolbar-button works).

In theory this should also work in Thunderbird, but could not be tested unfortunately.
@ntninja
Copy link
Collaborator Author

ntninja commented Feb 10, 2017

I don't want to be annoying or anything but have you had time to take a look at this by now, @meh? In particular, since Firefox 51 has now been released, it'd be possible to release an intermediate version of this add-on that migrates the settings from the old (non-WebExtension) version of this add-on to the newly written code-base.
Do you think that would be worth the hassle to look into?

@ntninja
Copy link
Collaborator Author

ntninja commented Apr 2, 2017

Since somebody (@publicarray) asked about this, I think I will complete what I've started here.
Since Firefox 51 has already been out for some weeks, we can now use embedded WebExtensions to also do a proper preferences migration. Users should then be able to upgrade to this version without any loss of data.

@meh: Would you review and let me publish this upgrade on AMO?

manifest.json: request permissions to modify all webpages
main.js: fix not reading lowercase header.name
policy.js: fix invalid syntax
@ntninja
Copy link
Collaborator Author

ntninja commented Apr 2, 2017

OK, I think I'm done for now; awaiting any review and particularly some testing.

"Building" the code: Just run make xpi and use the result.

How to test the migration (please don't just "throw it in there", I need real feedback if things go wrong!):

  • You will need at least Firefox 51 or newer (the WebExtension itself is compatible with Firefox 45+, but the shim requires a newer version…)
  • Make a backup of the user.js file of your current Firefox profile (you'll need it if the migration fails)
  • Close the browser and launch it from terminal
  • Open the Browser-Console using Ctrl+Shift+J
  • Install the new version of the extension in the browser (you may need to disable xpinstall.signatures.required in about:config for this to work)
    • You should see messages from SmartReferer.LegacyPrefs in the terminal and stuff about a "preference migration" in the Browser-Console window
  • Open the add-on list (about:addons) and verify that all preferences have been properly migrated
  • Open the browser configuration (about:config) and verify that all smart-referer related preferences are gone
  • Use the network logs in the Browser-Console to verify that the Referer values are indeed updated as expected while browsing

If errors happen during the migration I will need both the relevant terminal and Browser-Console output!

@publicarray
Copy link
Contributor

publicarray commented Apr 4, 2017

I tested the migration from a clean FF profile. It works as expected however I did not follow your instructions as make xpi errored on my mac:

zip error: Nothing to do! (smart-referer.xpi)
make: *** [smart-referer.xpi] Error 12

From the browser console:

Attempting preference migration from legacy add-on…  main.js:73:3
Received preferences from legacy add-on:  main.js:75:4
Object { strict: true, mode: "direct", referer: "www.google.com", allow: "*.dev>* *.github.com>*.github.com", whitelist: "http://meh.github.io/smart-referer/…" }  main.js:76:4
Purging preferences from legacy add-on…  main.js:90:3
Preference migration successfully completed!  main.js:96:4

Legacy prefs are also gone 👏

@ntninja
Copy link
Collaborator Author

ntninja commented Apr 5, 2017

@publicarray: Thanks for your feedback!

Regarding the make xpi error:
Do have GIT installed and in your path on that device? It does actually require that to obtain the file list.
If you do, could you run this in your terminal and paste the result: git ls-files | grep -v '^[.]' | grep -vx 'Makefile' | grep -v '^webextension/deps/public-suffix-list'
Thanks!

@meh: What's your timeframe on providing initial feedback on this?

@ntninja ntninja deleted the wext branch April 17, 2017 11:06
@meh
Copy link
Owner

meh commented Apr 17, 2017

Published, now to wait for approval. Nice work! 🐼

@ntninja
Copy link
Collaborator Author

ntninja commented Apr 17, 2017

Thanks! 😁

@Gitoffthelawn
Copy link
Contributor

What's the current AMO queue position?

@meh
Copy link
Owner

meh commented Apr 23, 2017

Queue Position: 228 of 278

@Gitoffthelawn
Copy link
Contributor

@meh Thanks. Wow, it's possible humans will inhabit Mars before then. ;)

@publicarray
Copy link
Contributor

@meh may I ask for an update on the AMO queue position?

@meh
Copy link
Owner

meh commented May 13, 2017

@publicarray Queue Position: 159 of 287

@publicarray
Copy link
Contributor

publicarray commented May 13, 2017

Thanks! This looks like a race, what will come first? The FF update that disables the current add-on or the new shiny extension gets approved? ;)

@meh
Copy link
Owner

meh commented May 13, 2017

Yeah, this is the first time it takes so long to get approved, it usually got put in front of the queue, probably because they were just minor updates.

@ntninja
Copy link
Collaborator Author

ntninja commented May 13, 2017

Actually the currently generated extension isn't even compatible with 57+, since it includes an unconfined migration aid -.-

The queue we're waiting in for Smart Referer is also not the only one. After submitting an update to a new (from-scratch, WExt-only, 57+) extension, it's currently number 604 of 662 (at a about 15 progress per day) in it's queue… Probably just about every currently maintained add-on in existence sits in that queue!
I guess guys from Mozilla are seriously drowning right now, not just are the review queues overflowing: All those missing APIs (both for feature-parity with Chrome and for allowing lots other stuff that is currently possible [toolbars, vertical tabs, accessing TLS data, managing webPage localStorage, … just to name a few]) need to be available in v55 the latest, so that people have at least the remaining 3 months for porting and getting their stuff through review. Firefox for Android is also currently a nightmare to program for and progress towards feature-parity with desktop Firefox is happening a snail's pace.

/So much for my rant on the current state of affairs in add-on-land. Just in case anybody wants to know how bad things currently are…

@reallyuniquename
Copy link

Could you please share XPI file of the latest version? I don't care if it's signed ot not.

@publicarray
Copy link
Contributor

@reallyuniquename I have made a singed version for myself (so that nightly stops complaining) with some changes. I'm willing to share it but be mindful that It's not 100% the code provided here.
key differences:

  • It does not contain code to migrate preferences from the old version.
  • I added a checkbox that allows the user to block same origin referrers as well. (I did not send a PR because this feature is was just for testing/advanced users and would break many sites i.e. not "smart")
  • won't auto-update because of the different ID
  • I suppose you also have to trust that I have not modified the extension for evil.

https://seby.io/smart_referer.xpi 19c592849b141132a747c5b3ec0941a7d1755abd

@reallyuniquename
Copy link

reallyuniquename commented May 14, 2017

@publicarray All right, thanks.

Few slightly unrelated questions.

Does this extension/version spoof document.referrer DOM property?

Can it be configured to spoof referer on direct page visit when browser sends no referer at all (e.g. bypassing hotlinking and 403s)? See #10.

@publicarray
Copy link
Contributor

Does this extension/version spoof document.referrer DOM property?

Yes, you can test it here https://seby.io/ref/ turn the Same Origin option on/off

Can it be configured to spoof referer on direct page visit when browser sends no referer at all (e.g. bypassing hotlinking and 403s)? See #10.

I think I just found a bug where the mode can not be changed 🙊. Normally setting the mode to "Send the URL you're going to as referer" should do what you want I think.

@ntninja
Copy link
Collaborator Author

ntninja commented May 14, 2017

Weird… where did my response go?

Anyway I tried to implement document.referrer support today and it took me a lot of time to figure out that rewriting the header apparently does change the value of that variable correctly as well (we never actually interact with the page content in our code, so this is a surprising finding that is likely very browser specific…). I'm also fairly certain that I had known this already some long time ago… 🙁

On the bright side: We got translation support and several other small fixes out of this (#54), so it at least was worth something 😄. (Also the current behaviour strikes make as quite a bit less than stable so the written code may still come in handy one day.)

@Gitoffthelawn
Copy link
Contributor

What's the current queue position?

@meh
Copy link
Owner

meh commented Jun 4, 2017

Queue Position: 40 of 195

@Gitoffthelawn
Copy link
Contributor

Thanks @meh

It's interesting how long they are taking.

But at their current rate, we should expect an official release within 2 weeks.

@publicarray
Copy link
Contributor

publicarray commented Jun 19, 2017

Any day now? 🙏 or has the Queue Position gone to negative one (-1) 😝

@meh
Copy link
Owner

meh commented Jun 19, 2017

Queue Position: 23 of 109

@publicarray
Copy link
Contributor

Thanks @meh

It's so close I can almost taste it.

@publicarray
Copy link
Contributor

publicarray commented Jun 22, 2017

@meh since we have been waiting for so long it is possible to ask at https://discourse.mozilla-community.org/c/add-ons/addons-mozilla-org if someone can look into it. This comment is also interesting.

Edit: If I had to guess than I think because of the major code changes to make this a web extension a reviewer flagged it for admin review which takes longer.

@meh
Copy link
Owner

meh commented Jul 8, 2017

It's been approved.

@Gitoffthelawn
Copy link
Contributor

Finally! But why is AMO not listing it as a webextension?

@ntninja
Copy link
Collaborator Author

ntninja commented Jul 8, 2017

Because it is an embedded WebExtension with an non-sandboxed migration agent. We'll have to upload an update by the beginning of September that drops the migration agent, then it will receive the 57+ badge. You can already build a pure-WebExtension version of the extension by running web-ext build in the webextension sub-directory; the migration agent is a purely optional component already. 🙂

@Gitoffthelawn
Copy link
Contributor

Thanks @Alexander255

@Gitoffthelawn
Copy link
Contributor

Gitoffthelawn commented Sep 27, 2017

@Alexander255 Is it time to drop the migration agent?

@ntninja
Copy link
Collaborator Author

ntninja commented Sep 27, 2017

@Gitoffthelawn Yes.

@meh Will you look into doing #55 in the next couple of days, or should we do another intrim release?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants