louisbarclay / nudge

Nudge makes the internet less addictive.
https://nudgeware.io
MIT License
143 stars 9 forks source link

Port to Mozilla Firefox #34

Open gadcam opened 6 years ago

gadcam commented 6 years ago

The API used in Firefox & Chrome is the same. Changes to port it to Firefox should be minimal as https://www.extensiontest.com/ says it is compatible with Firefox. If I can help you to port it just tell me how :)

louisbarclay commented 6 years ago

@gadcam This is really cool, thanks so much! I was just checking out the report and I do get quite a lot of warnings e.g. "Unsafe assignment to innerHTML", "Unsafe call to insertAdjacentHTML", "\"storage.sync\" can cause issues when loaded temporarily". It feels like these may stop many things about Nudge from working properly, so I'd have to fix them in order for the Firefox version to work - I don't think I can do that in the short term since I'm a bit under-resourced. I am very open to contributions if you think you'd ever want to take a look.

aadilayub commented 4 years ago

I've been trying to test this extension's compatibility for the last few hours, with little success. Currently. uploading the .crx on http://extensiontest.com on Chromium leads to this error:

image

On Firefox, it stays stuck at this screen for hours:

image

Hopefully using the web-ext cli should fare better. I'll report back with results.

aadilayub commented 4 years ago

Running web-ext run in the extension directory produces these results for me:

Running web extension from /home/aadil/nudge/extension
Use --verbose or open Tools > Web Developer > Browser Console to see logging

o: installTemporaryAddon: Error: unknownError: Could not install add-on at '/home/aadil/nudge/extension': Error: Extension is invalid
    at /home/aadil/.nvm/versions/node/v13.9.0/lib/node_modules/web-ext/dist/web-ext.js:1:5504
    at Client.handleMessage (/home/aadil/.nvm/versions/node/v13.9.0/lib/node_modules/web-ext/node_modules/@cliqz-oss/firefox-client/lib/client.js:162:7)
    at Client.readMessage (/home/aadil/.nvm/versions/node/v13.9.0/lib/node_modules/web-ext/node_modules/@cliqz-oss/firefox-client/lib/client.js:221:10)
    at Client.onData (/home/aadil/.nvm/versions/node/v13.9.0/lib/node_modules/web-ext/node_modules/@cliqz-oss/firefox-client/lib/client.js:187:16)
    at Socket.emit (events.js:321:20)
    at addChunk (_stream_readable.js:297:12)
    at readableAddChunk (_stream_readable.js:273:9)
    at Socket.Readable.push (_stream_readable.js:214:10)
    at TCP.onStreamRead (internal/stream_base_commons.js:186:23)
louisbarclay commented 4 years ago

@aadilayub thanks for doing this. This issue on web-ext repo shows a way to get more information using something called web-ext lint: https://github.com/mozilla/web-ext/issues/1077

If you are able to do that and put the results here it would be great. From what I recall of looking into porting to Firefox, there are a few awkward differences in APIs which might break Nudge. But I'd be willing to work through these.

aadilayub commented 4 years ago

@lgwb89 thanks for the tip! Here's the web-ext lint output.

Most of it is warnings. I'll paste the errors here:


Code                                 Message       Description              File          Line   Column
JSON_INVALID                         "/incognit…   Your JSON file could                                
                                     o" should     not be parsed.                                      
                                     be equal to                                                       
                                     one of the                                                        
                                     allowed                                                           
                                     values                                                            
MANIFEST_BACKGROUND_FILE_NOT_FOUND   A             Background script        manifest.j…                
                                     background    could not be found at    son                        
                                     script        "js/vars/credentials.…                              
                                     defined in    js".                                                
                                     the                                                               
                                     manifest                                                          
                                     could not                                                         
                                     be found.    

Not sure why the first error doesn't specify a file.

th0rgall commented 4 years ago

I want to point out that the Siempo browser extension which is AFAIK a (discontinued?) port of this extension does run on Firefox. The committers @kbot983 & @sythex may be able to comment on what they did to make that work.

I tried a while ago to import the Nudge extension code directly in Firefox after fixing some critical manifest values as posted above by @aadilayub. (well fixing, I think by just removing them). While Nudge then loaded without error, its features did not work. Don't remember the specifics though.

aadilayub commented 4 years ago

@th0rgall thanks for mentioning that. Don't know why I didn't think of looking at the siempo code, since your issue in their repo is what led to me to using nudge.

I ran web ext lint on the siempo extension, and got almost the same output, except there was no manifest.json error.

Here's the full output for anyone who wants to see it.

louisbarclay commented 4 years ago

Huge thanks @th0rgall and @aadilayub.

1) JSON_INVALID will be due to Nudge using the split value for incognito key in the manifest, which Firefox does not support. I will read up on what the different options are for that key and probably switch to one that Firefox does support.

image

2) MANIFEST_BACKGROUND_FILE_NOT_FOUND is due to an Amplitude credentials file not being on this repo (it's in .gitignore) - I will read up about how secure it would be to include it, or consider alternatives.

Once these errors are fixed, I suspect there will still be some API compatibility issues to be worked out.

I should note that I'm not actively seeking to work on porting Nudge to Firefox, although the interest here is definitely providing lots of votes for that and I will start considering this seriously.

louisbarclay commented 4 years ago

Update: I have changed the incognito value to spanning, which Firefox supports. Will continue thinking about the Amplitude credentials issue.

aadilayub commented 4 years ago

Just to add: I ran a diff on the manifest.json of Nudge and Siempo's fork, and it looks like skipping amplitude.js and credentials.js made their extension work on firefox.

image

That should confirm that it is the Amplitude credentials file creating the issues.

However, their version also doesn't seem to have the scroll nudger and a few other newer features, so those could still potentially be a source of API compatibility issues.

image

aadilayub commented 4 years ago

Update: I tried to sideload the extension on Firefox by omitting credentials.js and amplitude.js from the manifest.

The extension runs, but it gets stuck on this screen (can't add sites, behavior is described in #70):

image

I also couldn't see the list of sites that's usually there in the sites chooser, where the sites are categorised by "social", "news", etc).

louisbarclay commented 4 years ago

Update: I have changed the incognito value to spanning, which Firefox supports. Will continue thinking about the Amplitude credentials issue.

Update: unfortunately I had to change the incognito value back to split. spanning doesn't work well in Google Chrome. Specifically, the extension in Incognito is unable to access pages of the extension, so for instance the Defaulter page is broken because Incognito Nudge can't access the slider page that it needs to show. Pretty annoying behaviour but there it is.

So if you are porting Nudge across to Firefox/Brave you might want to make a script to change the incognito value to something else.

aadilayub commented 4 years ago

Testing on Firefox today (using the spanning incognito value) and I'm getting a TypeError at the sites chooser

sites.js:150:7
TypeError: settingsLocal.nudge_domains is undefined

image

himynameisdave commented 2 years ago

Thanks to all for all the work which has been done investigating the compatibility here.

Has anyone opened a PR / is there anything I could do to push this effort forward, as I am a Firefox user but would love to be able to use this extension.

th0rgall commented 2 years ago

Hey @himynameisdave! I haven't followed this since commenting, but I don't see a related PR and looking briefly at the forks there doesn't seem to be any Firefox-fixing going on elsewhere. So yes, if you feel like digging into web extension code, this is something you could help pushing forward. Some ideas:

1. Test incognito behavior again

Testing whether the compatibility issues found above regarding the incognito key are still the same, since the last activity here was over a year ago & browser features may have changed since.

2. Further debugging

Debug the issues with unknown causes found by @aadilayub https://github.com/louisbarclay/nudge/issues/34#issuecomment-600686643

3. Set up a simple build system

Only after doing (1) & (2) and seeing if this is still necessary. Referring to Louis' comment: https://github.com/louisbarclay/nudge/issues/34#issuecomment-621771430

So if you are porting Nudge across to Firefox/Brave you might want to make a script to change the incognito value to something else.

If the same manifest can't be used for all browser, manifest files need to be customized per browser. Plenty of other extensions do this. Here's an example of the toggl-button extension that uses webpack as a build system: https://github.com/toggl/toggl-button/blob/master/webpack.config.js#L100-L108

However, using webpack for this extension that is mostly consisting of vanilla JS seems overkill. To stay consistent with the rest of the code I would suggest writing a simple Node.js script that:

  1. Copies the extension code from /extension into a /dist/firefox and /dist/chrome folder
  2. Writes a custom manifest.json in each browser dist folder.

As a reference, I recently wrote a Node.js script in this project that writes Google API credentials https://github.com/louisbarclay/nudge/pull/122/files#diff-5bc58c8762730dba27675768ec55e0df069e97197a41cd1e16d31650a1d82cee.

I'm pretty sure these would be helpful, and you might get the extension working for yourself in Firefox. Once it's working, it's easier to get it published in the Firefox Add-on store.

louisbarclay commented 2 years ago

My master plan for doing this is to fork https://github.com/abhijithvijayan/web-extension-starter and rebuild Nudge from the ground up using it.

Nudge is due a rebuild anyway - the code is terrible, since it was mostly written when I was far less experienced as a developer.

I can't be certain on timing but will aim for the next 6 months.

fapdash commented 2 years ago

I can recommend https://github.com/webextension-toolbox/webextension-toolbox for cross-browser web extension development. Works great for our extension and it's actively maintained again since this year.

Would be great to use nudge in Firefox, thank you for all your work! @louisbarclay :)