olsh / Feedly-Notifier

Google Chrome, Firefox, Opera and Microsoft Edge extension for reading news from RSS aggregator Feedly
http://olsh.github.io/Feedly-Notifier/
Mozilla Public License 2.0
273 stars 38 forks source link

Compatibility with multi-process Firefox #63

Closed kiritsuku closed 7 years ago

kiritsuku commented 7 years ago

This plugin is not yet fully compatible with multi-process Firefox.

https://www.arewee10syet.com/ marks it as compatible-webextension but inside of Firefox is is marked as not compatible: ff

Daktyl198 commented 7 years ago

This should happen in the next release, if Olsh finishes turning it into a WebExtension (which, by definition, are e10s compatible).

Don't know when that will be, though.

olsh commented 7 years ago

I've alredy converted it into a WebExtension. But there is a bug with the panel, that's why I don't publish it to store. I hope the bug will be fixed in FF 51. If you want to test it, there are instuctions how to build the extension. https://github.com/olsh/Feedly-Notifier/blob/master/README.md You have to specify --browser firefox.

Keith94 commented 7 years ago

Hi @olsh I tested 2.12.1 in Firefox, but the options page has missing text for me, is it a known issue?

olsh commented 7 years ago

Hi @Keith94 No, this is not a known issue.

Did you build it with this command? grunt sandbox --clientId=sandbox --clientSecret=R26NGS2Q9NAPSEJHCXM3 --browser firefox

Please note that you have to specify the build folder when loading the extension locally.

Keith94 commented 7 years ago

Oh sorry for the trouble, I was not able to use bower install in the Git Shell, it seems I had to use npm install -g bower beforehand. Now it works.

Is there an easy way to package the files into an XPI for installing?

olsh commented 7 years ago

Not sure, but I suppose that there is no such way, because a WebExtension is just an archive. https://developer.mozilla.org/en-US/Add-ons/WebExtensions/Publishing_your_WebExtension

Daktyl198 commented 7 years ago

AFAIK, all you have to do is rename the .zip to .xpi and as long as you're running dev edition or nightly you should be able to install the extension.

kiritsuku commented 7 years ago

FF 51 has been released. Does the bug you were talking about still exist?

olsh commented 7 years ago

@sschaef Yes, the bug has been fixed in FF 51. I've published the WebExtensions version in AMO, it'll be available after AMO review.

kiritsuku commented 7 years ago

The most actual version is still 2.11.2: https://addons.mozilla.org/de/firefox/addon/feedly-notifier/ Did you publish somewhere else or are the newer versions still caught in review?

olsh commented 7 years ago

The addon didn't pass the review yesterday:

This version didn't pass review because of the following problems: 1) This add-on is creating DOM nodes from HTML strings containing potentially unsanitized data, by assigning to innerHTML, jQuery.html, or through similar means. Aside from being inefficient, this is a major security risk. For more information, see https://developer.mozilla.org/en/XUL_School/DOM_Building_and_HTML_Insertion . Here are some examples that were discovered:

  • scripts/options.js line 84
  • popup.html line 71 you are using {{{ templates, which will render any html. As this html can come from a remote source, it would give that page access to the popup's context, which is not safe. Please sanitize the html received here, for example using a library like DOMPurify.

I'll be on vacation for 2 weeks without internet and PC, sorry guys. I'll fix it only in the mid of March.

nagato7 commented 7 years ago

Any update on e10s support?

olsh commented 7 years ago

The review is too slow.

screenshot_032217_102130_am

olsh commented 7 years ago

The new version is on AMO. Please let me know if everything OK with e10s.

kiritsuku commented 7 years ago

It works really great. Thanks for all of the work Oleg!