Bugzilla – Full Text Bug Listing
| Summary: | [Proxy] add UI support to see what the automatically detected proxy settings are | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Francis Upton IV <francisu> | ||||||||||||
| Component: | Team | Assignee: | Pawel Pogorzelski <pawel.pogorzelski1> | ||||||||||||
| Status: | RESOLVED FIXED | QA Contact: | |||||||||||||
| Severity: | enhancement | ||||||||||||||
| Priority: | P3 | CC: | Kevin_McGuire, krzysztof.daniel, Szymon.Brandys | ||||||||||||
| Version: | 3.4 | ||||||||||||||
| Target Milestone: | 3.5 M4 | ||||||||||||||
| Hardware: | All | ||||||||||||||
| OS: | All | ||||||||||||||
| Whiteboard: | |||||||||||||||
| Attachments: |
|
||||||||||||||
|
Description
Francis Upton IV
Created attachment 97571 [details] Patch for UI and underlying changes This depends also on the patch for bug 226462. This adds a "show" button to the proxy preferences that shows the current detected system values. It also changes the language slightly in the existing system proxy selection button tooltip. Me to review. Francis, this patch is incomplete. ProxyData#getSource which should be in this patch, was added to another one and I didn't commit it. I believe it is better to have in a patch only changes that are required to fix the bug and I avoid adding things that possibly can be useful somewhere else. Anyway, I noticed that your changes contain new API and because we are past M6 we can't use it now. I think that we could create an example that contains dialog or view to test proxy, which will not be a part of SDK but will be available for people. The patch was complete with the original patches for bug 226462, but as you said, you removed those changes. This patch does not affect the API (I would not have submitted for M7 if it did), ProxyData is an internal class (note that I did not change IProxyData). I strongly feel this patch is important for supportability in 3.4, and realize that it is now past the functional complete milestone M7, so I'm not sure what the process is to get it in. I will revise the patch to make it self contained. It was difficult to make the original patch self contained since it was tied up with the other two bug reports for the same work. (In reply to comment #4) > The patch was complete with the original patches for bug 226462, but as you > said, you removed those changes. Right. I explained why, in my previous comment. > This patch does not affect the API (I would not have submitted for M7 if it > did), ProxyData is an internal class (note that I did not change IProxyData). Right. I noticed that you use x-friends. > I will revise the patch to make it self contained. This would be the best. > I strongly feel this patch is important for supportability in 3.4, and realize > that it is now past the functional complete milestone M7, so I'm not sure what > the process is to get it in. One comment about the patch. AFAIS it will present correctly only statically defined proxies. If we worked with dynamic stuff, the presented information would not be complete. I think that this is a polishing issue. The information about system proxies is rather harsh now and we need to have a nicer message. (In reply to comment #5) > > One comment about the patch. AFAIS it will present correctly only statically > defined proxies. If we worked with dynamic stuff, the presented information > would not be complete. > > I think that this is a polishing issue. The information about system proxies is > rather harsh now and we need to have a nicer message. > I'm not sure what you mean by this. The gnome proxy configuration for example is dynamic, when you change the settings in gnome, and then use the UI in this patch, you can see the updated settings. The UI will get the current settings from the environment. Is this what you mean, or something different? (In reply to comment #6) > I'm not sure what you mean by this. The gnome proxy configuration for example > is dynamic, when you change the settings in gnome, and then use the UI in this > patch, you can see the updated settings. The UI will get the current settings > from the environment. Is this what you mean, or something different? > So, even on gnome you can set PAC script (javascript file) as a source of proxy settings. It returns proxies relying on a given URL. So you can get different proxy settings for the same protocol but different hosts. (In reply to comment #7) > (In reply to comment #6) > > I'm not sure what you mean by this. The gnome proxy configuration for example > > is dynamic, when you change the settings in gnome, and then use the UI in this > > patch, you can see the updated settings. The UI will get the current settings > > from the environment. Is this what you mean, or something different? > > > > So, even on gnome you can set PAC script (javascript file) as a source of proxy > settings. It returns proxies relying on a given URL. So you can get different > proxy settings for the same protocol but different hosts. > I did not try setting the PAC script on gnome, but I did try modifying the proxy settings in the preferences (changing the host/port, turning of the use of proxies, etc) and the modifications that I made were reflected in the Eclipse UI when I reopened the dialog (as they should be because it calls the proxy provider to get the current values and it reads them from gconf which is dynamically updated). I'm not sure how the PAC stuff works with Gnome, if it sets the gconf settings based on the contents of the Javascript file, then it should update and we should see the new values (I suspect this is what it does). If it relies on the consumer of the configuration (in this case Eclipse) to figure out the Javascript file, then it will have no effect (we don't do anything to look for the values in this file). My implementation of the Gconf/Gnome stuff has the same functionality as what is built into the more recent versions of Java; whatever they did, that's what we do. When I revise the patch I will see how it handles the PAC file. Created attachment 98755 [details] Revised patch to have all changes in one place Since this touches the UnixProxyProvider, it also has the changes for bug 230040 included, so hopefully that will be applied first. Created attachment 115783 [details]
Screenshot
Is this correct that the same proxy setting is displayed three times?
Maybe we do not need new dialog? Could not we just reorder options:
* Direct connection to the Internet
* System proxy configuration
* Manual proxy configuration
Then we could optionally put the settings into the border and:
(a) disable it when direct connection is selected
(b) prefill it with system settings when system proxy is selected, but still user should not be able to edit those settings
(c) enable it for edition, and place default values when the user wants to configure it manually.
I could imagine even hiding all those setting fields when direct connection is selected, and displaying it only when b or c occurs.
what do you think?
(In reply to comment #10) > what do you think? > I think these ideas are good. The original patch was created quickly to get something out there for a UI, your ideas certainly are better. Keep in mind however that we probably want to be flexible in the UI to allow for changes in the proxy providers without having to alter the UI. I don't remember all of the details of this, but the same UI needs to work with the Windows and Unix proxy providers and probably other proxy providers in the future. System proxy configuration cannot share the same UI with manual configuration. This is because information about the proxy source has to be displayed per protocol. For Linux it is possible to set HTTP proxy as an environment variable and still have other protocols defined in GNOME. Bug 255616 has been created as a part of work originated from this bug. Created attachment 118282 [details]
Patch_v01
Patch that introduces the new proxy UI. Any feedback is welcome...
Comments: - add class headers to new classes (e.g. ProxyEntryDialog) -.classpath should not be modified - NonProxyHostsComposite contains unused import - ProxyEntryDialog line 199 -> warning - on Windows, check "Use the same proxy server for all protocols" in the Windows OS proxy dialog. When I open the settings page in Eclipse you will not see any native entries. Created attachment 118360 [details] Patch_v02 I've addressed first four comments. The fifth one is not connected with the patch provided, I opened a separate bug to track the issue. See bug 255959 for details. Ok. Looks good. Released to HEAD. |