Adds basic static black- and whitelist checking #60
Conversation
|
How complex are you imagining this logic will become? Would it make more sense to have a separate content script which registers with the whitelisted pages? https://developer.chrome.com/extensions/content_scripts#registration |
|
I don't expect the hardcoded lists to be used extensively, especially not the whitelist. This is intended more as a workaround until the detection logic can be properly fixed. But i'd like to add UI to allow users to add these workarounds themselves, and to eventually be able to report them back to us for fixing. As for the pattern matching itself, I don't expect anything crazier than wildcard matching. |
| Js.log @@ "mightBeOcamlDoc: " ^ (string_of_bool @@ mightBeOcamlDoc ()); | ||
| Js.log @@ "isBlackListed: " ^ (string_of_bool @@ isBlacklisted ()); | ||
| Js.log @@ "shouldConvert: " ^ (string_of_bool (isWhitelisted () || (mightBeOcamlDoc () && not (isBlacklisted ())))); | ||
| isWhitelisted () || (mightBeOcamlDoc () && not (isBlacklisted ())); |
There was a problem hiding this comment.
Can we make sure these (especially mightBeOcamlDoc) only get called once?
There was a problem hiding this comment.
Once per what? It could of course be cached in some storage area, but the chrome API is async and would make the logic messier (though if we just cache shouldConvert it wouldn't be as bad), and dom/localStorage APIs aren't isolated.
There was a problem hiding this comment.
Once per page load - right now mightBeOcamlDoc gets called three times, right? No need to cache across visits.
There was a problem hiding this comment.
It's just twice I think? One for ContentLoader to check if Content should be loaded, and then in Content itself. The simplest way I guess would be to just add a flag to window. Would that be ok you think?
Edit: Oh, it's actually four times with the logging, that could be improved of course :)
| Js.log @@ "isWhitelisted: " ^ (string_of_bool @@ isWhitelisted ()); | ||
| Js.log @@ "mightBeOcamlDoc: " ^ (string_of_bool @@ mightBeOcamlDoc ()); | ||
| Js.log @@ "isBlackListed: " ^ (string_of_bool @@ isBlacklisted ()); | ||
| Js.log @@ "shouldConvert: " ^ (string_of_bool (isWhitelisted () || (mightBeOcamlDoc () && not (isBlacklisted ())))); |
There was a problem hiding this comment.
Really like these logs for dev mode and am happy to leave them in - can you verify that the console logs get stripped in production? I don't want to force people to see this on every page load.
There was a problem hiding this comment.
I can verify that it does not get stripped. I could hide them behind a debug flag perhaps, for easier debugging in the wild.
glennsl
force-pushed
the
black-white-list
branch
from
8d19f1e
to
1e983e6
Compare
|
Ok, I've rebased, commented out the logging and "memoized" the shouldConvert call. |
Fixes #17
Fixes #40
Kind of fixes #46
Workaround for #49
Constitutes phase 1 of the masterplan for #48