search

tosdr / browser-extensions

Browser extensions for Terms of Service; Didn't Read. “I have read and agree to the Terms” is the biggest lie on the web. We aim to fix that. Get informed instantly about websites' terms & privacy policies, with ratings and summaries from the www.tosdr.org initiative.
http://tosdr.org/downloads
GNU Affero General Public License v3.0
393 stars 44 forks source link

Extension crashes Firefox when trying to find ToS;DR for tosdr.org #27

Closed jynnie closed 3 years ago

jynnie commented 5 years ago

When I attempt to open the extension while on tosdr.org, Firefox crashes.

michielbdejong commented 5 years ago

Hm, which version of Firefox do you have? I use Firefox 63 on Ubuntu, and I didn't see it crash, but I did see a spinnner that did not disappear like it should: screenshot from 2018-12-10 09-51-34

jynnie commented 5 years ago

I'm using Firefox 63.0.3 on Mac. I also get the spinner, but once I click off or close the pop-up, it instantly crashes.

jynnie commented 5 years ago

I also just updated to Firefox 64 to check if that fixed the issue, but it still crashes.

oslego commented 5 years ago

This is caused by invoking an invalid CSS selector due to the semicolon in the ToS;DR service name concatenated in the query selector expressions used with jQuery in popup.js

I encounter this on the Chrome extension on this page: https://edit.tosdr.org/ It's likely the same issue with Firefox.

Screen Shot 2019-06-08 at 14 50 27

As an example: try to run this query selector command in plain JS in any page to observe the error.

document.querySelector('#popup-point-ToS;DR-4518')

// VM26:1 Uncaught DOMException: Failed to execute 'querySelector' on 'Document': '#popup-point-ToS;DR-4518' is not a valid selector.
//    at <anonymous>:1:10

The getServiceDetails() promise does not catch the error thrown by tosdrPoint(), so the popup document remains stuck in the "loading" state.

Perhaps the serviceName should be sanitized when generating the element ID & querying for it. Or maybe dropping the serviceName altogether from the element ID if there's no real need for that distinction in the popup.html. The dataPoint.id should suffice to serve as a unique ID that's unique per service and services shown one at a time in the popup, right?

JustinBack commented 3 years ago

Fixed in 89e56898271bc1ec6404d46b12f100a9ca636e77