Closed Bug 202636 Opened 21 years ago Closed 21 years ago

document.referrer has no properties for XML documents

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: pratik.solanki, Assigned: pratik.solanki)

Details

Attachments

(1 file, 2 obsolete files)

Spun off from bug 192366. Looking at page info for any XML document always shows
"no referrer" as the referrer field. It should show the correct referrer.
jst:

You had some thoughts on how to do this. Can you tell me what you said on IRC
(since I've forgotten what you said)? If we can come up with a design no how to
do this, then I can write up the code without much effort.
Simply add a "readonly attribute DOMString referrer" attribute to
nsIDOMNSDocument, and implement that in nsDocument, and you should be all set.
This attribute will be shadowed by nsIDOMHTMLDocument::referrer on HTML
documents, but that's ok, they're the same thing so it doesn't matter through
which interface you get the referrer.
Oh, and move the referrer code (except the getter) from nsHTMLDocument to
nsDocument.
Attached patch patch, v1 (obsolete) — Splinter Review
Here's my first stab at this patch. I tested it with XML and XUL URLS and it
works.
Attachment #139400 - Flags: review?(jst)
Comment on attachment 139400 [details] [diff] [review]
patch, v1

- In content/base/public/nsIDocument.h:

+  nsCString mReferrer;

This new member should go into nsDocument, not into nsIDocument.

+NS_IMETHODIMP
+nsDocument::GetReferrer(nsAString& aReferrer)
+{
+  CopyUTF8toUTF16(mReferrer, aReferrer);
+
+  return NS_OK;
+}
+
+nsresult
+nsDocument::SetReferrer(const nsAString& aReferrer)
+{
+  CopyUTF16toUTF8(aReferrer, mReferrer);
+  return NS_OK;
+}
+
[...]
+    // The misspelled key 'referer' is as per the HTTP spec
+    rv = httpChannel->GetRequestHeader(NS_LITERAL_CSTRING("referer"),
+					 mReferrer);
(fix the indentation of the second line in that call)
+    if (NS_FAILED(rv)) {
+      mReferrer.Truncate();
+    }
+    

Now that you're adding this code to nsDocument, you can remove all the referrer
code from nsHTMLDocument (since that now inherits the code from nsDocument).
IOW, remove mReferrer from nsHTMLDocument, and the code that's not needed there
any more.

Fix that, and I'll have one more look.
Attachment #139400 - Flags: review?(jst) → review-
Thanks for the comments. I'll fix up my patch over the weekend.
Attached patch patch, v2 (obsolete) — Splinter Review
Second try. I got rid of some of the referrer code from nsHTMLDocument but
can't seem to remove Get/SetReferrer. So I'm just calling
nsDocument::Get/SetReferrer().

Questions
1. Should nsDocument::SetRefferer be declared as an NS_IMETHOD?
2. My previous patch used CopyUTF16toUTF8/CopyUTF8toUTF16 but somehow I started
geting errors with those functions so I'm now using
CopyUCS2toASCII/CopyASCIItoUCS2. Let me know if which is the right function to
call.
Assignee: general → psolanki
Status: NEW → ASSIGNED
Comment on attachment 139875 [details] [diff] [review]
patch, v2

+nsDocument::GetReferrer(nsAString& aReferrer)
+{
+//  CopyUTF8toUTF16(mReferrer, aReferrer);
+  CopyASCIItoUCS2(mReferrer, aReferrer);

Use the UTF8 version (it should compile, I can't see why it wouldn't here.

+NS_IMETHODIMP
+nsDocument::SetReferrer(const nsAString& aReferrer)
+{
+//  CopyUTF8toUTF16(aReferrer, mReferrer);
+  CopyUCS2toASCII(aReferrer, mReferrer);
+  return NS_OK;
+}

Seems like this method isn't needed, I don't see anyone calling it (except
nsHTMLDocument::SetReferrer, which can probably be removed too, see below).

- In nsDocument.h:

+  /**
+   * Set the referrer of this document.
+   */
+  NS_IMETHOD SetReferrer(const nsAString& aReferrer);

This should be removed too, unless it turns out to be needed.

 NS_IMETHODIMP
 nsHTMLDocument::SetReferrer(const nsAString& aReferrer)
 {
-  mReferrer.Assign(aReferrer);
-
-  return NS_OK;
+  return nsDocument::SetReferrer(aReferrer);
 }

The only reason this is needed here is that the SetReferrer() method is defined
in the interface nsIHTMLDocument, but I don't see anyone calling that method,
so it seems like you should be able to remove the method from nsIHTMLDocument,
and then eliminate it from nsHTMLDocument as well, and thus also from
nsDocument.

- In nsHTMLDocument.h:

-  nsString mReferrer;
+  //nsString mReferrer;

Just remove this alltogether, don't just comment it out.

Fix that, and this should be ready to go (assuming I'm correct on that
nsIHTMLDocument::SetReferrer() is not needed any more).
Attachment #139875 - Flags: review-
Attached patch patch, v3Splinter Review
Third try.
Attachment #139400 - Attachment is obsolete: true
Attachment #139875 - Attachment is obsolete: true
Comment on attachment 139950 [details] [diff] [review]
patch, v3

Very nice! r+sr=jst
Attachment #139950 - Flags: superreview+
Attachment #139950 - Flags: review+
Fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Now that we got rid of RetrieveRelevantHeaders() from nsHTMLDocument, we don't
need to declare it as a virtual function in nsDocument. Can we just drop the
virtual attribute? No one else overrides it (according to lxr).
Yup, good catch! Fix checked in.
This caused bug 233197 -- some of the code removed from
nsHTMLDocument::RetrieveRelevantHeaders was not placed anywhere else...
Oops.. Gotta be more careful. Thanks for taking care of this bz.
Component: DOM: Mozilla Extensions → DOM
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: