foxyproxy / browser-extension

Version 8 and above. Browser extension source code for Firefox, Chrome, and other Chromium-based browsers
GNU General Public License v2.0
228 stars 33 forks source link

icon doesn't change when proxy by pattern matches #57

Closed tessus closed 9 months ago

tessus commented 9 months ago

Previously the icon changed and showed the proxy that was used when browsing. This was very helpful as I was able to monitor what was going on.

Now it only shows

image

Any chance you can bring back that functionality? If not, is there a way to install the old version?

erosman commented 9 months ago

Performance

Unreliable & Unusable Information

Scenario 1

The final result provides no reliable information. All it says is that something on the tab was proxied by some proxy. It would get more complicated and unreliable when more domains are included in a page.

Scenario 2

Once again, the final result provides no reliable information and can create a false impression.

Other Network Requests

installgentoo commented 9 months ago

It's actually fairly useful to see which proxies are firing, if you have multiple proxies set up.

Could keep the badge as sort of a statistic, once per second(or however often changing it would be negligible to performance)? Just keep the last n requests in a vector, then randomly sample from the vector and set badge to that. This will tend to display whichever proxies are most used, but also will have the chance to display all proxies being used.

Alternatively, keep a table of hits on last used distinct proxies and iterate through it every second, changing the badge and resetting number of hits to 0.

erosman commented 9 months ago

It's actually fairly useful to see which proxies are firing, if you have multiple proxies set up.

That wont be possible to see since it can flash 10s of times per second.

Could keep the badge as sort of a statistic, once per second(or however often changing it would be negligible to performance)?

Not possible as it is tied to the web requests.

Could keep the badge as sort of a statistic, once per second(or however often changing it would be negligible to performance)? Just keep the last n requests in a vector, then randomly sample from the vector and set badge to that. This will tend to display whichever proxies are most used, but also will have the chance to display all proxies being used.

Not possible for a toolbar badge. However, on Firefox, the live Log is there for this purpose and it has extensive data.

installgentoo commented 9 months ago

Yes, but you have to access live log, wherein icon is always visible.

Isn't it possible to have a 1 sec timer and do the operation on a table, subsequently changing the badge, and to write to that table after each request?

Alternatively, can do you have access to some sort of timeout function in the extension code? Just skip changing the badge, only write statistics, then on a request after 1sec timeout, check the table, change the badge, and reset timeout to 1sec.

erosman commented 9 months ago

Isn't it possible to have a 1 sec timer and do the operation on a table, subsequently changing the badge, and to write to that table after each request?

That wont solve the issue. A timer simply postpones the process i.e. 100s of requests will be processed X seconds later. If a table is created, it has to be overwritten with every request which would make it useless.

It is not possible to display a table on badge text or title.

In any case, it wont solve the issues outline in https://github.com/foxyproxy/browser-extension/issues/57#issuecomment-1848266511

sidneys commented 9 months ago

Hi,

Wouldn't making this (IMHO very important ) UI feature optional – with a disclaimer referring to the expected performance impact – be a valid strategy?

Cheers!

tessus commented 9 months ago

I also think that making this optional would be the best solution. The code is available (from version 7) and thus it shouldn't be very time consuming to make it available again.

I certainly do understand the performance impact and why it was removed, but it was very useful and gave me piece of mind, just to have a visual cue that a proxy is used.

mnalis commented 9 months ago

I too much prefer previous version...

Combined with losing all the data from #53, I've gone back to previous version thanks to this Reddit thread post, which solved both issues in my Firefox. So I'm sticking to that 7.5.1 and disabling updates for now.

(Now would be a good time to do that "Export" just in case...)

ssuesskind commented 9 months ago

I too much prefer previous version

Personally, I also find it very hard to fathom that this was an intended change.

It represents a HUGE downgrade in functionality and usability.

tessus commented 9 months ago

@erosman sorry man. it seems I opened a can of worms here. This was not my intention.

erosman commented 9 months ago

@erosman sorry man. it seems I opened a can of worms here. This was not my intention.

No problem.... Any issue can be discussed logically.

I had another look at the FoxyProxy v7 code.

https://github.com/foxyproxy/firefox-extension/blob/a3598e1c7f0237ee50e8a13a59dec265462c38a6/src/scripts/utils.js#L217-L238

The setting of the icon was not tab specific. It was for all the tabs. Every time a proxy was used (anywhere, in any tab), it would change the icon for all the tabs.

That would mean ...

How would that be useful? Wouldn't that give the user the wrong impression that example.con is also being proxied?

tessus commented 9 months ago

Well, yes and no. For me it is important to see the badge change when I open a tab and load a page. e.g. I usually don't have X tabs open that all have constant concurrent connections going on.

I am fully aware of the situation and what you are saying. But for me, it is good enough to open a new tab, load a page and see the icon change. Boom, I am happy. Same thing when I reload the page.

Switching tabs, having X tabs that all have concurrent connections, this is another story and that doesn't bother me. I expected as much when I was using version 7.x. I am aware of it. All good.

ShayArtzi commented 9 months ago

+1 to @tessus, I also miss the quick visual feedback about active proxy being used.

There might be some ways to mitigate some of the concerns shared above (debouncing/making the feature optional/show the indicator in the popup menu instead of changing the icon frequently and so on) and find a way to bring this feature back.

I would also like to take this opportunity to thank everyone involved in making this great browser extension! Happy holidays everyone :)

klemens commented 9 months ago

One possible implementation might be to only show the proxy used for the top level document in the tab. For third-party requests, a "+" symbol or something like that could be added to indicate that there were third-party requests that use a different connection than the one shown currently.

You could then even show more details in the pop-out menu when clicking the icon. All of that state should of course be stored per tab (think uBlock Origin which always shows request details for the currently active tab).

asia-by-jalopy commented 9 months ago

I also miss the quick visual feedback about active proxy being used.

+1. I know it is not perfect. I know it has flaws. I know performance impact. I willingly accepted all of those negative things when I used older FoxyProxy version. I still loved it. Please bring it back?

redelsoft commented 9 months ago

I'm also missing the visual feedback on the FP icon on which proxy is being used. Now I have to check the logs every time I've doubt is a proxy is being used. So a +1 on that feature becoming available again.

erosman commented 9 months ago

I'm also missing the visual feedback on the FP icon on which proxy is being used. Now I have to check the logs every time I've doubt is a proxy is being used. So a +1 on that feature becoming available again.

The icon change shows that proxy is being used somewhere. It doesn't show if the proxy is being used in the current tab.

It can come from a net usage of:

It doesn't pin point which proxy is being used and where either. It is just a blinker and that blinks when proxy is being used. :smile: Users can only see the text for the LAST change clearly.

However, if there is considerable demand for it, we can re-add the feature.

tessus commented 9 months ago

It doesn't show if the proxy is being used in the current tab.

But it could, couldn't it? There are add-ons that change the icon based on the tab. e.g. Tab reloader.

So why not make the icon change dependent on the current tab being used? But even, if that's too much work to implement, just bringing back the previous behavior (optional and with a warning in the docs) would already be more than sufficient.

erosman commented 9 months ago

But it could, couldn't it?

Yes, it can, but that wasn't the case in the pre-8 versions. :wink: Frankly, the work is not much different if it is global or per tab.

:pushpin: The concern is about changing the badge, color, title 100 times (that is 100x3) when a tab loads while the user does not benefit at all from 99 of them and can only see or read the last one. Wouldn't that be a 99% waste?

It is possible to limit it to the top frame (not change when inner frames are proxied), but that can also lead to confusion.

tessus commented 9 months ago

Wouldn't that be a 99% waste?

It would, but there's not much you can do about it from a technical perspective. It is what it is. You can't change the requests when loading a page or only change the badge for the last one. But that's what happened in 7.x, so this unnecessary changing is nothing new. And if the consequences are described in the docs, it is everyone's prerogative to use it or not.

superschalk commented 9 months ago

asia-by-jalopy commented Dec 13, 2023

+1. I know it is not perfect. I know it has flaws. I know performance impact. I willingly accepted all of those negative things when I used older FoxyProxy version. I still loved it. Please bring it back?

I wholeheartedly agree 👍 The dynamic icon was one of the most useful aspects of this extension.

erosman commented 9 months ago

Fair enough... I will work on the toolbar icon badge indicator, when in "Proxy by Patterns" mode.

erosman commented 9 months ago

v8.7 Added option to proxies on the toolbar badge when in Proxy by Patterns mode (Firefox only)

Beta updated on repo for testing. Enable "Show Pattern Proxy" to test.

tessus commented 9 months ago

Thanks for the update!

Beta updated on repo for testing.

What does that mean? There's no draft release, not did I see anything on the website and/or the Mozilla add-on page.

Update: Ahh, I see. I have to build it myself.

ssuesskind commented 9 months ago

erosman commented v8.7 Added option to proxies on the toolbar badge when in Proxy by Patterns mode (Firefox only)

Beta updated on repo for testing. Enable "Show Pattern Proxy" to test.

The feature doesn't work for me (v8.7 Beta).

But even normal proxying does not work anymore on 8.7.

Reverted back to v8.6 from the Mozilla Extension Store (proxying again works).

Tested: Via manifest.json install & locally packaged (via web-ext) .zip. Browser: Firefox Nightly 122.0a1

erosman commented 9 months ago

Thank you for testing. I will fix the issue and update the repo.

erosman commented 9 months ago

v8.7 on repo updated with fix for testing.

Note: The badge is per-tab

ssuesskind commented 9 months ago

v8.7 on repo updated with fix for testing.

The latest v8.7 commit seems to be fixed 😄 The automatic icon indicator is back, and the correct dark mode icon is implemented.

Great work!

1 3 2
erosman commented 9 months ago

Foxy Proxy v8.7 uploaded to AMO. Basic is available now and standard will be after it is reviewed by AMO.