Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
tessus opened this issue Dec 9, 2023 · 29 comments
Closed

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

tessus opened this issue Dec 9, 2023 · 29 comments
Labels
area: user-interface UI issues discussion Needs further discussion done ✓

Comments

@tessus
Copy link

tessus commented Dec 9, 2023

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 erosman added the area: user-interface UI issues label Dec 9, 2023
@erosman
Copy link
Collaborator

erosman commented Dec 9, 2023

Performance

  • Browsers often make 100s of network connections in a second
  • Setting the toolbar badge is an asynchronous operation
  • Each setting involves 3 separate asynchronous operation to set text, background-color, & title
  • Rapidly changing the toolbar badge impacts browser performance
  • The impact of extensions on browser performance are monitored by the browsers
  • Rapidly changing the toolbar badge causes the badge to flash repeatedly
  • There have been users who complained about the rapid flashing

Unreliable & Unusable Information

  • Network connection requests in each tab often comes from multiple (10s of) sources and domains
  • The final badge can only reflect the last asynchronously processed request

Scenario 1

  • foo.com is set to go through proxy A
  • bar.com is set to go through proxy B
  • foo.com page has an image from bar.com
  • When going to foo.com, 2 network requests are made
    • if the connection to foo.com is processed first, badge flashes A and settles on B
    • if the connection to bar.com is processed first, badge flashes B and settles on A

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

  • foo.com is set to go through proxy A
  • bar.com is set to go through directly
  • foo.com page has an image from bar.com
  • When going to foo.com, 2 network requests are made
    • if the connection to foo.com is processed first, badge flashes A and settles on no-proxy
    • if the connection to bar.com is processed first, badge settles on A

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

Other Network Requests

  • In addition to the network requests made by the pages, there are other background connections made by the browser and other extensions
  • These connections are proxied but they don't relate to any tab
  • Changing the tab badge will provide confusing and unusable information

@installgentoo
Copy link

installgentoo commented Dec 9, 2023

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
Copy link
Collaborator

erosman commented Dec 9, 2023

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
Copy link

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
Copy link
Collaborator

erosman commented Dec 9, 2023

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 #57 (comment)

@sidneys
Copy link

sidneys commented Dec 9, 2023

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
Copy link
Author

tessus commented Dec 9, 2023

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
Copy link

mnalis commented Dec 9, 2023

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
Copy link

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
Copy link
Author

tessus commented Dec 10, 2023

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

@erosman
Copy link
Collaborator

erosman commented Dec 10, 2023

@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 ...

  • User loads abc.com which is proxied, icon changes to show it is proxied
  • User switches to example.con which is not proxied
  • The icon remains the same and showing the proxy server

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

@tessus
Copy link
Author

tessus commented Dec 10, 2023

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
Copy link

+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
Copy link

klemens commented Dec 10, 2023

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).

@erosman erosman added the discussion Needs further discussion label Dec 11, 2023
@asia-by-jalopy
Copy link

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
Copy link

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
Copy link
Collaborator

erosman commented Dec 13, 2023

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:

  • another extension
  • browser
  • another tab
  • some element (not all) on the current tab

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. 😄
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
Copy link
Author

tessus commented Dec 13, 2023

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
Copy link
Collaborator

erosman commented Dec 13, 2023

But it could, couldn't it?

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

📌 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
Copy link
Author

tessus commented Dec 13, 2023

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
Copy link

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
Copy link
Collaborator

erosman commented Dec 14, 2023

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

@erosman
Copy link
Collaborator

erosman commented Dec 15, 2023

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
Copy link
Author

tessus commented Dec 15, 2023

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
Copy link

ssuesskind commented Dec 15, 2023

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
Copy link
Collaborator

erosman commented Dec 15, 2023

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

@erosman
Copy link
Collaborator

erosman commented Dec 15, 2023

v8.7 on repo updated with fix for testing.

Note: The badge is per-tab

@ssuesskind
Copy link

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
Copy link
Collaborator

erosman commented Dec 16, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: user-interface UI issues discussion Needs further discussion done ✓
Projects
None yet
Development

No branches or pull requests