553970 - nsIURL::GetRelativeSpec documentation is incorrect

Status: RESOLVED FIXED

(Core :: Networking, defect)

Description

[Justin Lebar (not reading bugmail)]

In nsIURL.idl:

    /**
     * This method takes a uri and returns a substring of this if it can be
     * made relative to the uri passed in.  If no commonality is found, the
     * entire uri spec is returned.  If they are identical, "" is returned.
     * Filename, query, etc are always returned except when uris are identical.
     */
    AUTF8String getRelativeSpec(in nsIURI aURIToCompare);

Note that the documentation says that the return value is a substring of |this|.

Test case:

  nsCOMPtr<nsIURI> uri1, uri2; 
  NS_NewURI(getter_AddRefs(uri1), 
            NS_LITERAL_STRING("http://foo.com/")); 
  NS_NewURI(getter_AddRefs(uri2), 
            NS_LITERAL_STRING("http://foo.com/bar")); 
  nsCOMPtr<nsIURL> url1(do_QueryInterface(uri1)), 
                   url2(do_QueryInterface(uri2)); 
 
  nsCAutoString rel; 
  url1->GetRelativeSpec(url2, rel); 
  printf("url1->GetRelativeSpec(url2) = '%s'\n", rel.get()); // prints "bar"
  url2->GetRelativeSpec(url1, rel);
  printf("url2->GetRelativeSpec(url1) = '%s'\n", rel.get()); // prints ""

Comment 1

[Christian :Biesinger (don't email me, ping me on IRC)]

Hm, yeah. It's incorrect anyway in that it doesn't necessarily return a substring (it may add some ../)

Comment 2

[Justin Lebar (not reading bugmail)]

Attachment: Patch

Christian, what do you think of this comment?

Comment 3

[Justin Lebar (not reading bugmail)]

Attachment: Testcase

This is the code I used to check the comment.

Comment 4

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 434016 [details] [diff] [review]
Patch

Can you turn your C++ testing code into an xpcshell test for netwerk/test/unit?

Comment 5

[Justin Lebar (not reading bugmail)]

Attachment: Patch with testcase

Comment 6

[Justin Lebar (not reading bugmail)]

Attachment: Patch with testcase v1.1

'this' -> '|this|' in the comment, which is clearer, I think.

Comment 7

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 434547 [details] [diff] [review]
Patch with testcase v1.1

I think makeURL is a more common name for this kind of function

Comment 8

[Justin Lebar (not reading bugmail)]

Attachment: Patch for checkin

Comment 9

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/282d7009f226