Closed Bug 553970 Opened 13 years ago Closed 13 years ago

nsIURL::GetRelativeSpec documentation is incorrect

Categories

(Core :: Networking, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9.3a4

People

(Reporter: justin.lebar+bug, Assigned: justin.lebar+bug)

Details

Attachments

(1 file, 4 obsolete files)

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 ""
Hm, yeah. It's incorrect anyway in that it doesn't necessarily return a substring (it may add some ../)
Summary: nsIURL::GetRelativeSpec documentation inverts its parameters → nsIURL::GetRelativeSpec documentation is incorrect
Attached patch Patch (obsolete) — Splinter Review
Christian, what do you think of this comment?
Attachment #434016 - Flags: review?(cbiesinger)
Attached file Testcase (obsolete) —
This is the code I used to check the comment.
Attachment #434020 - Attachment mime type: application/octet-stream → text/plain
Comment on attachment 434016 [details] [diff] [review]
Patch

Can you turn your C++ testing code into an xpcshell test for netwerk/test/unit?
Attachment #434016 - Flags: review?(cbiesinger) → review+
Attached patch Patch with testcase (obsolete) — Splinter Review
Attachment #434016 - Attachment is obsolete: true
Attachment #434020 - Attachment is obsolete: true
Attached patch Patch with testcase v1.1 (obsolete) — Splinter Review
'this' -> '|this|' in the comment, which is clearer, I think.
Assignee: nobody → justin.lebar+bug
Attachment #434546 - Attachment is obsolete: true
Status: NEW → ASSIGNED
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
Attachment #434547 - Flags: review+
Attachment #434547 - Attachment is obsolete: true
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/282d7009f226
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a4
You need to log in before you can comment on or make changes to this bug.