interlock-network / threatslayer

A Chrome browser extension that protects you from malicious sites.
GNU General Public License v3.0
12 stars 3 forks source link

Blair/sync urlscan worigin #73

Open blairmunroakusa opened 1 year ago

blairmunroakusa commented 1 year ago

Simplified contribution to head dev branch to,

jmercouris commented 1 year ago

Have you confirmed that all operations are still functional :-D?

blairmunroakusa commented 1 year ago

@jmercouris @DecentralizedDan I have officially covered my tracks and found all the broken things I thought I found but didn't. ! I fixed them. I have list of bugs I collected to diff against my branch. That is as issue here: PR https://github.com/interlock-network/threatslayer/issues/75

@DecentralizedDan The issues you were having with the scripting API breaking things if not in manifest has something to do with how you didn't complete the merge somehow...not sure what happened, but after inspecting remove-seed-phrase, I couldn't find any changes relevant to this PR. That is good though, because I made a final update to this PR to make the improved TS version non-crap and fully functional.

This branch is ready-ready for merge.

jmercouris commented 1 year ago

Let us be careful to make so many changes right before a big release! It could be catastrophic :-O

blairmunroakusa commented 1 year ago

I mean, we don't need to merge now...makes no difference to me ultimately in the near term. In any case however, our merge into main will introduce massive changes regardless of this particular PR being included or not. I am only proposing a merge into that branch itself that we will soon merge to main, this PR actually a merge to main. The merge 1) works 2) simplifies everything 3) cleans up the mess that is the extension src dir and 4) uses fewer permissions than the previous remove-seed-phrase branch.

Just saying this to emphasize that the large changes in PR are comparatively speaking, a non issue. We could compromise, and create a branch out of current remove-seed-phrase to merge this into, so we can see how it handles. My only gripe is that the extension as it stands is 1) a mess 2) uses more permissions than it needs and 3) is garbage in the way the url scanning flow is implemented. Like, it's embarrassing (and I feel somewhat responsible, because it was I who implemented that particular flow for triggering the warning banner).

But yeah, whatever, not really important to me at the moment. If however, I find myself needing to build dApp stuff into our service-worker or our banner content script, then for the record I need this branch integrated with main.

blairmunroakusa commented 1 year ago

And then there is the messed up styling problem: image image

Above are a couple examples of how current remove-seed-phrase branch messes up styling after closing warning banner.

This branch fixes that.

jmercouris commented 1 year ago

I guess I'm confused. How is this related to a dApp?

jmercouris commented 1 year ago

Are you saying that there exists some sort of integration with Polkadot that is misbehaving given the current environment?