Closed
Bug 500983
Opened 15 years ago
Closed 14 years ago
"Use System Proxy Settings" should be default for new profiles.
Categories
(Core :: Networking, defect)
Core
Networking
Tracking
()
RESOLVED
FIXED
mozilla1.9.3a1
People
(Reporter: andrewshilliday, Assigned: andrewshilliday)
References
Details
Attachments
(1 file)
1.21 KB,
patch
|
bzbarsky
:
review+
beltzner
:
approval1.9.2.4+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1) Gecko/20090624 Firefox/3.5 Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1) Gecko/20090624 Firefox/3.5 With 1.9.1, OS X (and Linux) users have the option to have Firefox use the system proxy settings instead of manually setting them to mimic whatever settings are used. This has the advantage that if users alter their system proxy settings (e.g., change network location), Firefox automatically makes the switch to the new settings. When new users install Firefox, the default setting is "no proxy". Once this new feature of using system settings is fully implemented and tested (dependency on bug 457377), it should be the default setting for new users (i.e., new profiles). Reproducible: Always
Comment 1•15 years ago
|
||
Do we really have to limit it to one OS or can we have this advantage on all platforms?
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: wanted1.9.1.x?
Flags: blocking1.9.1?
OS: Mac OS X → All
Version: unspecified → Trunk
Comment 2•15 years ago
|
||
There is no way this is going to block the release of Firefox 3.5 at this point, and you should know that. Please don't just flip every flag to get driver attention.
Flags: blocking1.9.1? → blocking1.9.1-
Comment 3•15 years ago
|
||
Windows doesn't have a "system proxy" for Firefox 3.5, but it's available on the trunk (see bug 485764) Note: bug bug 457377 is only for the SOCKS proxies
Comment 4•15 years ago
|
||
(In reply to comment #2) > There is no way this is going to block the release of Firefox 3.5 at this > point, and you should know that. Please don't just flip every flag to get > driver attention. Sorry, that was by accident. I wanted to flag blocking1.9.2.
Flags: blocking1.9.2?
Assignee | ||
Comment 5•15 years ago
|
||
I'd agree this should be for all systems and not just OSX just wasn't sure "use system settings" was available to windows users. Thanks Jo.
Updated•15 years ago
|
Flags: blocking1.9.2? → blocking1.9.2-
Assignee | ||
Comment 6•15 years ago
|
||
Removed the ifndef/ifdef conditions so that option is default for all platforms. I'm not sure if there is a way to make this still conditional on whether the systemproxy component is available for the platform, anyone have any thoughts?
Attachment #392739 -
Flags: review?(beltzner)
Updated•15 years ago
|
Attachment #392739 -
Attachment mime type: application/octet-stream → text/plain
Updated•15 years ago
|
Attachment #392739 -
Attachment is patch: true
Updated•15 years ago
|
status1.9.1:
--- → ?
Flags: wanted1.9.1.x?
This fix will be the only way we can use Firefox at our school. Currently "use system settings" must be configured for each user!!!!!! There's no way we would take the time to log in as each user and set this option. Our content filtering is managed via a proxy server. I can't let students or teachers use Firefox without this setting in place. It's far easier to deny access to Firefox, as unfortunate as that is. Most of our users prefer Firefox and that's the only browser working for Teacher Web which our teachers utilize for updating their web pages. I certainly hope something can be done, as this is a MAJOR pitfall for us.
Comment 8•15 years ago
|
||
Comment on attachment 392739 [details] [diff] [review] Makes "use system settings" default option for all platforms Actual review needs to be done by a viable reviewer.
Attachment #392739 -
Flags: ui-review?(beltzner)
Attachment #392739 -
Flags: review?(bzbarsky)
Attachment #392739 -
Flags: review?(beltzner)
Updated•15 years ago
|
Component: General → Networking
QA Contact: general → networking
Comment 9•15 years ago
|
||
So how does the "5" setting behave if we in fact do not have a systemproxy component?
Comment 10•15 years ago
|
||
(In reply to comment #9) > So how does the "5" setting behave if we in fact do not have a systemproxy > component? This line is called both in nsProtocolProxyService::Init() and when the network.proxy. prefs branch changes: http://mxr.mozilla.org/mozilla1.9.1/source/netwerk/base/src/nsProtocolProxyService.cpp#408 do_GetService() is called with "@mozilla.org/system-proxy-settings;1" Not sure what do_GetService() does when the service isn't available. Does it throw?
Comment 11•15 years ago
|
||
It returns null.
Comment 12•15 years ago
|
||
Comment on attachment 392739 [details] [diff] [review] Makes "use system settings" default option for all platforms r- pending either an explanation of why this behaves correctly (identically to not using a proxy) on setups where there is no system proxy settings component or changes to the code to make it do so.
Attachment #392739 -
Flags: review?(bzbarsky) → review-
Comment 13•15 years ago
|
||
Comment on attachment 392739 [details] [diff] [review] Makes "use system settings" default option for all platforms I don't think this needs a ui-review; if the default change is idempotent in terms of user experience (ie: it still works) then fine. Sounds like Boris needs convincing, though!
Attachment #392739 -
Flags: ui-review?(beltzner)
Comment 14•15 years ago
|
||
In the interest of adding more detail on how this affects users, here's what I recently experienced: 1. A friend wanted to install Firefox on her work computer. It was locked down, so I didn't know whether it would work, but it seems the work that has been done in this area with installing without admin privileges actually allows this. Awesome. 2. She proceeds to start Firefox, and doesn't want to import the bookmarks or anything else. I assume this also means that she didn't get any proxy settings imported. 3. When Firefox tries to load a page, it fails. We later find that this is because it's configured to have "No Proxy" instead of automatically detecting the proxy. Switching to Auto for proxy makes the browser work as it should. If I wasn't there to help, she probably would have said "Firefox doesn't work, I'll have to use IE" instead. Of course, making it work in settings where there are no proxies when it's set to "auto" is key. Not sure if it already does that, when we switched her to Auto, it worked on several non-work networks too.
Comment 15•15 years ago
|
||
I think we all understand why we want this fixed. I just want to make sure the code is correct in a particular edge case; it looks fine otherwise. It also looks like we need someone to pick up and own this bug...
Comment 16•15 years ago
|
||
Comment on attachment 392739 [details] [diff] [review] Makes "use system settings" default option for all platforms So. afaict, *most* of the code wants mSystemProxySettings to be null. Except. I believe this code: 1226 nsresult 1227 nsProtocolProxyService::Resolve_Internal(nsIURI *uri, 1228 const nsProtocolInfo &info, 1229 PRBool *usePAC, 1230 nsIProxyInfo **result) 1234 *usePAC = PR_FALSE; 1235 *result = nsnull; 1240 if (mSystemProxySettings) { 1242 if (NS_SUCCEEDED(mSystemProxySettings->GetPACURI(PACURI)) && 1247 if (NS_FAILED(rv)) 1248 return rv; here we have a mSystemProxySettings and are falling out of the if block to line 1260 1249 } else { 1251 nsresult rv = mSystemProxySettings->GetProxyForURI(uri, proxy); 1252 if (NS_SUCCEEDED(rv)) { 1253 ProcessPACString(proxy, result); 1254 return NS_OK; 1255 } 1256 // no proxy, stop search 1257 return NS_OK; 1258 } 1259 } 1260 1263 if (mProxyConfig == eProxyConfig_Direct || 1264 (mProxyConfig == eProxyConfig_Manual && 1265 !CanUseProxy(uri, info.defaultPort))) 1266 return NS_OK; 1268 // Proxy auto config magic... 1269 if (mProxyConfig == eProxyConfig_PAC || mProxyConfig == eProxyConfig_WPAD || 1270 mProxyConfig == eProxyConfig_System) { 1271 // Do not query PAC now. 1272 *usePAC = PR_TRUE; 1273 return NS_OK; here we've decided to usePAC even though we don't have a working systemproxy
Attachment #392739 -
Flags: review-
Comment 17•15 years ago
|
||
note bug 529773 (and its patch)
Comment 18•14 years ago
|
||
Comment on attachment 392739 [details] [diff] [review] Makes "use system settings" default option for all platforms The checked-in patch for bug 529773 addresses bz's concern, so I've requested re-review of the patch on this bug.
Attachment #392739 -
Flags: review?(bzbarsky)
Attachment #392739 -
Flags: review-
Comment 19•14 years ago
|
||
Comment on attachment 392739 [details] [diff] [review] Makes "use system settings" default option for all platforms r=bzbarsky
Attachment #392739 -
Flags: review?(bzbarsky) → review+
Updated•14 years ago
|
Assignee: nobody → andrewshilliday
Updated•14 years ago
|
Status: NEW → ASSIGNED
Keywords: checkin-needed
Comment 20•14 years ago
|
||
Pushed to m-c: http://hg.mozilla.org/mozilla-central/rev/9f2860ad939e
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a1
Comment 21•14 years ago
|
||
Comment on attachment 392739 [details] [diff] [review] Makes "use system settings" default option for all platforms Requesting approval1.9.2.1 as it's a safe change, and allows Firefox to "work" (without further configuration) when a user's system has proxy settings already configured.
Attachment #392739 -
Flags: approval1.9.2.1?
Comment 22•14 years ago
|
||
It's not a safe change unless bug 529773 is landed first. And it's not obvious to me how safe that is.
Comment 23•14 years ago
|
||
Ah I guess that landed on 1.9.2. OK.
Comment 24•14 years ago
|
||
Comment on attachment 392739 [details] [diff] [review] Makes "use system settings" default option for all platforms Moving nomination to 1.9.2.3, we'll take it when the tree re-opens.
Attachment #392739 -
Flags: approval1.9.2.2? → approval1.9.2.3?
Comment 25•14 years ago
|
||
Comment on attachment 392739 [details] [diff] [review] Makes "use system settings" default option for all platforms a=beltzner for 1.9.2.4
Attachment #392739 -
Flags: approval1.9.2.4? → approval1.9.2.4+
Comment 26•14 years ago
|
||
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/8419a9bf117e
status1.9.2:
--- → .4-fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•