wingman-jr-addon / wingman_jr

This is the official repository (https://github.com/wingman-jr-addon/wingman_jr) for the Wingman Jr. Firefox addon, which filters NSFW images in the browser fully client-side: https://addons.mozilla.org/en-US/firefox/addon/wingman-jr-filter/ Optional DNS-blocking using Cloudflare's 1.1.1.1 for families! Also, check out the blog!
https://wingman-jr.blogspot.com/
Other
35 stars 6 forks source link

Cached images are not checked #60

Closed abdullahezzat1 closed 4 years ago

abdullahezzat1 commented 4 years ago

Since the extension deals with the headers, the real images are actually cached by firefox, and when firefox uses cached images it never goes through the request/response cycle and so it's never detected by the extension.

I found a workaround is to go to about:config and set browser.cache.check_doc_frequency to 1 which makes the browser check for a new version of the content every time a page is loaded.

wingman-jr-addon commented 4 years ago

Thanks for the report!

I have more or less assumed that this isn't that big of a deal since it means that you would already have visited the site/image prior to this. But sometimes what I think isn't a big deal for me is a big deal for somebody else. I haven't looked into this too much. (I usually use shift+refresh to reload the page for testing). However, I do see MDN seems to address this a bit via webRequest.handlerBehaviorChanged. I could perhaps call this method once when the plugin is upgraded/installed as a guard against this.

@abdullahezzat1 What would you think of that type of solution? Also, is there a specific way that you use this plugin where the current behavior ends up being annoying?

abdullahezzat1 commented 4 years ago

webRequest.handlerBehaviorChanged should handle old cache that existed before installation/upgrade.

The other problem I mentioned is that the browser caches images before any modifications. Maybe setting the header Cache-Control: no-store would do the trick.

@wingman-jr-addon I think the current behavior works really well. I don't know a lot about machine learning, but I think the model may be missing some training for group photos. False positives are pretty low but the lower they get the better of course.

wingman-jr-addon commented 4 years ago

Hmm.... I guess I'd just have to try it to see. I thought all requests - even cached ones, other than the "in-memory cache" the handlerBehaviorChanged talks about - would route through webRequest. I guess if it's not a big concern maybe I'll implement the handlerBehaviorChanged and close it out if it's not a common use case.

@abdullahezzat1 I'd like to continue the discussion on the group photos, but on its own issue so we don't get this thread mixed in. See #61

wingman-jr-addon commented 4 years ago

@abdullahezzat1 I'd be curious for a few more details regarding how you reproduce this issue. I tried two distinct cases.

Primary Case

For the primary case, I did the following steps:

  1. Load a Google image search with something tame but a bit borderline.
  2. Optionally, scroll down a bit. Images on the first page are all data: urls, images after load dynamically.
  3. See images load normally.
  4. Install plugin.
  5. Hit refresh on the page. (not shift+refresh, just normal refresh)

What I expected was based on your bug report:

  1. No change in images displayed on page.

What I actually saw:

  1. Some images turned into blocked images as expected.

Secondary Case

As a secondary note, I did also try looking at images on their own page. Here is what I found:

  1. Load a Google image search with something tame but a bit borderline.
  2. Scroll past first page.
  3. Click on an image.
  4. Right click on the detailed image and choose "View Image" to take it directly to the URL.

At this point I expected based on my first set of results to:

  1. Install the plugin.
  2. Hit refresh.
  3. See image blocked.

What I actually saw was:

  1. Install the plugin.
  2. Hit refresh.
  3. Image remained unchanged.

So I tried add the webRequest.handlerBehaviorChanged. Then what I saw was:

  1. Install the plugin.
  2. Hit refresh.
  3. Image remains unchanged.

Hitting shift+refresh multiple times also failed. What seemed to work was hitting back then forward, then hitting shift+refresh.

Conclusion

So, unfortunately without more evidence here I was seeing that the primary case seemed to already work without webRequest.handlerBehaviorChanged and that the secondary case didn't work even with webRequest.handlerBehaviorChanged.

Given this is appearing to be a bit finicky and I don't think it's a common use case I will probably close it unless I get more details on ways the primary case fails. Please let me know what you see.

wingman-jr-addon commented 4 years ago

Last call before I close as cannot reproduce...

wingman-jr-addon commented 4 years ago

Closing because I could not reproduce the primary use case. Secondary use case I am judging to be too small to keep looking at.

wingman-jr-addon commented 3 years ago

@abdullahezzat1 I've been thinking about this further and just wanted to let you know that I ended up calling webRequest.handlerBehaviorChanged on processor page load now. 👍