-
Notifications
You must be signed in to change notification settings - Fork 9
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
Ported extension to the W3C Web Extensions API to support Firefox, Chrome, and more #43
base: master
Are you sure you want to change the base?
Conversation
…for "Open Website" button, breaking the extension.
Addressing Muffo#13. Also updated NPM packages to latest and fixed minor JSLint error in options.js.
Addressing Muffo#13. Also updated NPM packages to latest and fixed minor JSLint error in options.js.
Fixed issue where full article images use .lightbox class resulting in the image being absolutely positioned in the top left corner and overlaying all of the Feedly UI and content. Feedly also uses the .lightbox class, resulting in the mismatch. The fix involves removing all class and width attributes from all images in the full-article content and setting the style attribute to max-width:100% in order to keep larger images contained within the article container and flow.
Fixed issue where full article images use .lightbox class resulting in the image being absolutely positioned in the top left corner and overlaying all of the Feedly UI and content. Feedly also uses the .lightbox class, resulting in the mismatch. The fix involves removing all class and width attributes from all images in the full-article content and setting the style attribute to max-width:100% in order to keep larger images contained within the article container and flow.
* full-article-image-fix: Article image formatting fix
Replaced document with contentElement to contain image fixes to just the article being loaded.
Replaced document with contentElement to contain image fixes to just the article being loaded.
…l-article-image-fix
* spinjs-bower: Updated Spin.js to use Bower
# Conflicts: # app/manifest.json # bower.json
* commit 'a230c07991c8f71e58930d7aec2c2c90267dd535': Prepare for release 0.9.0 Correct missing spaces in comments Fixed bug causing formatting with other Feedly elements Article image formatting fix Move Mercury before Readability Add help image for Mercury Added support for Postlight Labs' Mercury Web Parser API
Wow this is great!! I'll take a look at the changes as soon as possible: I am really curious to know more about these new extension APIs. |
The changes are really pretty minor as the Web Extensions API is largely based on the existing Chrome extensions API. Besides renaming chrome.* to browser.*, the biggest change is the onMessage API becoming promise-based instead of callback-based. |
@@ -0,0 +1,5 @@ | |||
{ | |||
"tabWidth": 4, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is cool! I heard of this project but never had a chance to use it.
Does it actually re-format all the source files based on the style specified here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, if you run it at the command line on the project. There's also a great Visual Studio Code extension for it at https://marketplace.visualstudio.com/items?itemName=esbenp.prettier-vscode. It's great for making sure all of the files are formatted consistently.
@@ -143,7 +143,6 @@ module.exports = function (grunt) { | |||
dest: '<%= config.dist %>' | |||
}, | |||
html: [ | |||
'<%= config.app %>/popup.html', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good spot! I didn't realize this file was still there
"manifest_version": 2, | ||
"description": "__MSG_appDescription__", | ||
"icons": { | ||
"16": "images/icon-16.png", | ||
"128": "images/icon-128.png" | ||
}, | ||
"applications": { | ||
"gecko": { | ||
"id": "addon@fullyfeedly" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Out of curiosity, is it required to package the application for Firefox?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is more necessary to debug the application in Firefox since the local storage API requires the application ID to function. It's also required for publishing the extension to addons.firefox.com.
app/manifest.json
Outdated
"scripts/background.js" | ||
], | ||
"persistent": false | ||
"scripts": ["scripts/chromereload.js", "scripts/browser-polyfill.js", "scripts/background.js"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is polyfill used to provide backward compatibility to certain browsers?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. The polyfill is used to provide compatibility for browsers that implement the chrome.* APIs but not the replacement browser.* APIs (i.e. Chromium-based browsers). Browser.* support is coming in Chromium, but it's not available yet, so this enables the same code to work across browsers.
@@ -45,11 +46,13 @@ | |||
"options.html" | |||
], | |||
"permissions": [ | |||
"declarativeContent", | |||
"activeTab", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is activeTab
Chrome specific?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not, however Chrome and Firefox implement the permission differently. ActiveTab in Firefox doesn't give you access to the URL while it does in Chrome. Tabs gives access to the URL in both.
Replaced chrome.* APIs with browser.* APIs and added the webextension-polyfill to support browsers that don't support the browser.* APIs yet.
Tested in Firefox 57+ and Chrome 62+.
Should theoretically work in Edge, Opera, and Safari as well as any other browser that supports the W3C Web Extensions API.