438288 - URI.path one character short after redirect involving fragment

Status: VERIFIED FIXED

(Core :: Networking, defect, P2)

Description

[John P Baker]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9pre) Gecko/2008060906 Minefield/3.0pre
Build Identifier: 

URI.path is missing the last character of the fragment after a redirect - URI.spec is correct.

Reproducible: Always

Steps to Reproduce:
1. Install the attached extension
2. Go to http://www.meyerweb.com/#thoughts
   (redirects to http://meyerweb.com/#thoughts)
Actual Results:  
The path in the second alert is /#thought

Expected Results:  
The path should be /#thoughts on both alerts

Sorry that the test involves an extension; It is basically my WIP with an alert to debug it, and then the guts removed to just alert the spec and path, on history visit, of any URI involving a fragment.

Comment 1

[John P Baker]

Attachment: extension to alert path and spec on history visit

Comment 2

[Mats Palmgren (inactive)]

Attachment: Patch rev. 1

Comment 3

[Lucas Malor (mail: c6kfnkn2uc AT snkmail DOT c0m)]

Severity to Major?

The bug affects also Fx2? Otherwise Version must be changed to 3.0 or Trunk.

I request Blocking (or else Wanted) opinion.

Comment 4

[Mike Beltzner [:beltzner, not reading bugmail]]

Can we get an answer to what versions this bug touches?

Comment 5

[Mats Palmgren (inactive)]

(In reply to comment #4)
> Can we get an answer to what versions this bug touches?

Code inspection suggests all versions since 2001-11-15.
I tested it occurs in 2.0.0.14, but I haven't tested older builds.
Rev. 1.1:
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/netwerk/base/src/nsStandardURL.cpp&rev=1.1&root=/cvsroot&mark=1374,1376,1380#1345
Rev. 1.110 (CVS tip):
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/netwerk/base/src/nsStandardURL.cpp&rev=1.110&root=/cvsroot&mark=2205,2207,2222#2171

Comment 6

[Mats Palmgren (inactive)]

http://hg.mozilla.org/mozilla-central/index.cgi/rev/f0aec487ed11

-> FIXED

Comment 7

[Mats Palmgren (inactive)]

Comment on attachment 324466 [details] [diff] [review]
Patch rev. 1

Do we want this on branches?

Comment 8

[Samuel Sidler (old account; do not CC)]

Comment on attachment 324466 [details] [diff] [review]
Patch rev. 1

Approved for 1.9.0.2. Please land in CVS. a=ss

Comment 9

[Mats Palmgren (inactive)]

Landed in CVS trunk:
mozilla/netwerk/base/src/nsStandardURL.cpp 	1.111
mozilla/netwerk/test/unit/test_standardurl.js 	1.3 

Comment 10

[Daniel Veditz [:dveditz]]

Comment on attachment 324466 [details] [diff] [review]
Patch rev. 1

Approved for 1.8.1.17, a=dveditz for release-drivers

Comment 11

[Mats Palmgren (inactive)]

Landed on MOZILLA_1_8_BRANCH:
mozilla/netwerk/base/src/nsStandardURL.cpp 	1.82.4.12 

(netwerk/test/unit/test_standardurl.js doesn't exist on this branch
so I couldn't apply the unit test part of the patch)

Comment 12

[Al Billings [:abillings - ex-MoCo]]

(In reply to comment #0)
> Steps to Reproduce:
> 1. Install the attached extension
> 2. Go to http://www.meyerweb.com/#thoughts
>    (redirects to http://meyerweb.com/#thoughts)

This extension is Firefox 3+ only. We need a way to verify this for 2.0.0.17, which includes this fix.

Comment 13

[Mats Palmgren (inactive)]

Attachment: unit test JS

Comment 14

[Mats Palmgren (inactive)]

Attachment: test-harness-head.js

Comment 15

[Mats Palmgren (inactive)]

Attachment: unit test HTML

This is what I used to test 1.8 branch:
1. save this HTML file
2. save first JS file in the same directory, name it "438288.js"
3. save second JS file to "test-harness-head.js"
4. edit the file: URL in the HTML file to where "test-harness-head.js"
   is located on your system

Comment 16

[Al Billings [:abillings - ex-MoCo]]

I am assuming a negative proof here? Using the above files, 2.0.0.16 states that the testcase failed in an alert. 2.0.0.17 doesn't say anything, it just renders an empty window without the alert. Is this correct?

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.17) Gecko/2008082909 Firefox/2.0.0.17

Comment 17

[Mats Palmgren (inactive)]

Correct.

Comment 18

[Al Billings [:abillings - ex-MoCo]]

Verified then. Whew.

Comment 19

[Aakash Desai [:aakashd]]

verified FIXED on builds: 
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090414 Minefield/3.6a1pre ID:20090414030735

and

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b4pre) Gecko/20090414 Shiretoko/3.5b4pre ID:20090414035212