network.proxy.autoconfig_url.include_path=true does not work

VERIFIED FIXED in Firefox 52

Status

()

VERIFIED FIXED
2 years ago
2 years ago

People

(Reporter: alice0775, Assigned: bagder)

Tracking

({regression})

51 Branch
mozilla54
x86
Windows 10
regression
Points:
---

Firefox Tracking Flags

(firefox-esr45 unaffected, firefox51 wontfix, firefox52+ verified, firefox-esr52 fixed, firefox53 fixed, firefox54 verified)

Details

(Whiteboard: [necko-active])

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
Steps To Reproduce:
1. save proxy.pac file with the following contents

function FindProxyForURL(url, host) { 
alert(url);
return "DIRECT";
}

2. Set network.proxy.autoconfig_url.include_path=true in about:config
3. about:preferences#advanced > Network > Settings...
   turn on "Automatic proxy configuration URL:" 
    file:///C:/path/to/proxy.pac

4. Open Browser Console
5. Load any page (e.g. http://forums.mozillazine.org/viewforum.php?f=38 )
   ---- observe log

Actual Results:
network.proxy.autoconfig_url.include_path=true does not work.
The url is stripped.
PAC-alert: http://forums.mozillazine.org/

Expected Results:
network.proxy.autoconfig_url.include_path=true should work.
The following should be logged.
PAC-alert: http://forums.mozillazine.org/viewforum.php?f=38
Daniel, is this a regression from recent changes?
Assignee: nobody → daniel
Whiteboard: [necko-active]
(Reporter)

Comment 2

2 years ago
[Tracking Requested - why for this release]: it is worth to fix the issue for next ESR 52

Regressed by Bug 1255474
tracking-firefox52: --- → ?
Keywords: regression
(Assignee)

Comment 3

2 years ago
When toggling this option, you need to restart Firefox for it to "take effect". Did you do that?
Flags: needinfo?(alice0775)
(Reporter)

Comment 4

2 years ago
(In reply to Daniel Stenberg [:bagder] from comment #3)
> When toggling this option, you need to restart Firefox for it to "take
> effect". Did you do that?

Yes, I restart Firefox.
Flags: needinfo?(alice0775)
(Assignee)

Comment 5

2 years ago
Ugh. I think I spotted the mistake. I'll try a patch and report back soonish.
(Assignee)

Comment 6

2 years ago
Created attachment 8835900 [details] [diff] [review]
0001-bug-1336069-init-class-member-before-return-r-mcmanu.patch

This tiny change fixes the problem for me. The initialization of the class member in the Init() method was accidentally put after the code path that normally returns early so it remained with its default value (false) all the time!
Attachment #8835900 - Flags: review?(mcmanus)
Comment on attachment 8835900 [details] [diff] [review]
0001-bug-1336069-init-class-member-before-return-r-mcmanu.patch

Review of attachment 8835900 [details] [diff] [review]:
-----------------------------------------------------------------

doh!
Attachment #8835900 - Flags: review?(mcmanus) → review+
(Assignee)

Updated

2 years ago
Keywords: checkin-needed

Comment 9

2 years ago
Pushed by mozilla@noorenberghe.ca:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8a2f028e6943
init class member before return, r=mcmanus
Keywords: checkin-needed

Comment 10

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/8a2f028e6943
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox54: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Please nominate for aurora/beta uplift when you get a chance.
tracking-firefox52: ? → +
Flags: needinfo?(daniel)
(Assignee)

Comment 12

2 years ago
Comment on attachment 8835900 [details] [diff] [review]
0001-bug-1336069-init-class-member-before-return-r-mcmanu.patch

Approval Request Comment
[Feature/Bug causing the regression]: 1336069
[User impact if declined]: "network.proxy.autoconfig_url.include_path" won't work as documented
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: 
[List of other uplifts needed for the feature/fix]:
[Is the change risky?]: No
[Why is the change risky/not risky?]: the change is minuscule, easy to review and in code not frequently changed
[String changes made/needed]: N/A
Flags: needinfo?(daniel)
Attachment #8835900 - Flags: approval-mozilla-beta?
Attachment #8835900 - Flags: approval-mozilla-aurora?
Hi Brindusa, could you help find someone to verify if this issue was fixed as expected on a latest Nightly build? Thanks!
Flags: needinfo?(brindusa.tot)
Verified as fixed on latest Nightly 54.0a1, Build ID 20170215030342 using Windows 10.
Status: RESOLVED → VERIFIED
status-firefox54: fixed → verified
Flags: needinfo?(brindusa.tot)
Comment on attachment 8835900 [details] [diff] [review]
0001-bug-1336069-init-class-member-before-return-r-mcmanu.patch

let's take this regression fix in aurora53 and beta52

Should be in 52.0b7
Attachment #8835900 - Flags: approval-mozilla-beta?
Attachment #8835900 - Flags: approval-mozilla-beta+
Attachment #8835900 - Flags: approval-mozilla-aurora?
Attachment #8835900 - Flags: approval-mozilla-aurora+

Comment 16

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/f7a276a267cd
status-firefox53: affected → fixed

Comment 17

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/23b17b9d6a4e
status-firefox52: affected → fixed
Let's check this on Beta 52 as well.
Flags: qe-verify+

Comment 19

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-esr52/rev/23b17b9d6a4e
status-firefox-esr52: --- → fixed
Reproduced the initial issue using Firefox 51.0.1 on Windows 7 32bit, verified that this is fixed using Firefox 52 beta 8 on Windows 7 x32, Ubuntu 12.04 x32 and macOS 10.12.3.
status-firefox52: fixed → verified
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.