+ default zone option

[abdullahezzat1]

Created an option to set the default zone when the extension starts.

It works, but there's a small problem. Say I set the default zone to "untrusted", it changes successfully on the next start, but the icon changes to red, THEN comes back to white, even though the zone is still untrusted.

So, it's only an icon problem. I will leave that to you, since I couldn't figure it out.

Ignore the whitespace diffs.

Thanks.

[wingman-jr-addon]

Thanks @abdullahezzat1 ! It will be a little longer before I can get back to this. I took a brief look and I'm not 100% sure why you'd be seeing that visual issue either; guess I'll have to dig into it.

[abdullahezzat1]

What is the point of redrawing the icon anyway? Can't it just be solved by making template icons like it used to be? Besides, it's not that pretty in dark mode. It was way better before.

[wingman-jr-addon]

The icon has an animated status bar in the bottom left for image scanning and a video scanning indicator in the bottom right.

[abdullahezzat1]

The icon is too tiny to see progress, plus you don't really need progress, you can simply put a circle at the left corner as an indicator that the extension is currently processing image(s), and another circle on the right corner for video(s). Something like this:

You can then make templates:

• neutral_processing_images_only.png
• neutral_processing_videos_only.png
• neutral_processing_both.png
• neutral_no_processing.png And so on for all zones.

Then simply toggle between them based on the processing status.

[wingman-jr-addon]

Well, I guess I find progress to be helpful. I actually sunk quite a bit of time into making it work from the earlier static representation. For example, I find it helpful if I visit an image-heavy site like Pinterest and 100+ images queue up, it gives me a way to see what's going on. Processing takes long enough on my laptop that it's useful to have feedback beyond simply "processing" or "not processing". I use the muted tones so that the progress bar (hopefully) does not draw attention to itself unless you are looking for it. How quickly do image-heavy pages tend to scan for you? It could be that we just have different performance leading to different experiences here.

[abdullahezzat1]

I think it's time to consider separating development from production.

In a production version (release), you don't need all the processing of checking what's being processed at the moment, whether it's a progress bar or statically, at least in my opinion. Also, all the logging should be removed in production. Basically, saving all the processing power for the real purpose of the extension should be the priority in a production version.

For a pinterest page, average of 127ms (wasm only)

[wingman-jr-addon]

Do note we've gone beyond the original PR intent quite a bit. However, I appreciate the discussion.

To be clear about the progress bar: I enjoy the progress bar as a production feature. I disliked the lack of feedback I was getting during scanning on heavy pages. I see some type of a subtle progress indicator here as a good thing; it must be quite discreet in order to not distract the user. I'd be open to hearing other ideas on how to represent the progress indicator, but I do not wish to drop it.

Regarding logs and production: yes, you likely have a point here. There is a great deal of logging occurring and it is possibly affecting performance. Generally in compiled languages you'd tack on some type of conditional compilation step to ensure the logs don't get compiled in at all, but here I do not have a build system. I do not want a build system either because then I need to build before I can load and if I e.g. pack or minify, then it requires me to do more during my addon submission to AMO due to their requirements. So if I did do something, I'd want to have it be light and not require a build system.

Specifically for logging I think there are three things that could contribute to poor performance:

1. The actual log call e.g. console.log
2. Any work done to prepare the log statement e.g. string interpolation
3. Leaked memory due to the console hanging onto the objects

1 and 3 can be solved by simply providing no-op stubs for e.g. console.log. But 2 is much harder to satisfy without a compilation step. I think I would lean towards the simple solution that leverages the short-circuit operator:

let DEBUG = true / false
DEBUG && console.log('string')

Like from this StackOverflow post

Given the constraints of not introducing a build system, do you have a different recommendation for dealing with logs in production?

[abdullahezzat1]

The logging solution is good enough, I guess.

Regarding the progress bar thing, I would like to say that there will always be someone who doesn't like a certain feature, like me :relaxed:. If I were you, I would make it optional, not mandatory. I'm not going to bother you with this anymore, you're free to do what you want.

[abdullahezzat1]

Actually, regarding the log solution, a better answer is the one after you mentioned. This one

So, it's one condition at the start that can disable all the logging in production without changing any code.

[wingman-jr-addon]

Ah, that solution would be the "no-op stub" for console.log that I was talking about. One thing that happens with that solution is that you still have overhead related to bullet point 2, where you do the work of preparing the log statement. Any other ideas?

[abdullahezzat1]

No, the debug solution is enough.

[wingman-jr-addon]

https://github.com/wingman-jr-addon/wingman_jr/tree/set-log-to-debug

[wingman-jr-addon]

(FYI, logging branch is merged)

[wingman-jr-addon]

@abdullahezzat1 I stepped through the merge conflicts between the logging changes I made and the whitespace changes you made. Hopefully didn't break anything along the way, but take a look.

[wingman-jr-addon]

@abdullahezzat1 Thanks for the PR! Looks good. I did end up figuring out that zone visual glitch and put it in master on https://github.com/wingman-jr-addon/wingman_jr/commit/47f39be898cf3686a1402c5eee97a9015b74b43c