156786 - Referrer not set when opening links in new window

Status: RESOLVED FIXED

(Core Graveyard :: Embedding: Mac, defect)

Description

[Mike Pinkerton (not reading bugmail)]

- go to above page (caution, it's pr0n and may even disappear w/in a few days)
- use context menu to open any of the thumbnails in a new window

expected:
image appears in new window

actual:
because the referrer is not set, the webserver redirects you to a "not found" page.

this is the equiv of bug 156732, but for PPEmbed.

Comment 1

[Mike Pinkerton (not reading bugmail)]

nominating topembed since this impacts websurfing and website revenue generation
with sites that use the referrer trick.

Comment 2

[Matthias Versen [:Matti]]

*** Bug 156785 has been marked as a duplicate of this bug. ***

Comment 3

[Conrad Carlen (not reading bugmail)]

Got it.

Comment 4

[Jason Bassford]

Dupe of bug 121084?

Comment 5

[Mike Pinkerton (not reading bugmail)]

no, not a dupe.

Comment 6

[saari (gone)]

giving this topembed+ to preempt someone finding it later and screaming. Every
fix counts!

Comment 7

[Jonadab the Unsightly One (Nathan Eady)]

Summary does not correctly identify the issue.  Mozilla *does* send the
referer information when opening a link in a new window or a new tab.
This can be verified easily here:
http://cgi.galion.lib.oh.us/ip-fork.htm
Right-click the link and choose "New Window".  Alternately, create
a link to that script from any page anywhere, rinse, repeat.  The
referring page will be correctly identified by the script.  (There
is an exception; a file:/// address will NOT be sent, presumably 
for privacy reasons.)

Reporter, please test the simplified testcase here:
http://cgi.galion.lib.oh.us/ip-fork.htm

WORKSFORME, Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.1a) Gecko/20020610

Otherwise, please explain what beyond the referer is causing
the problem in question.

Comment 8

[Steve Dagley]

Jonadab - notice this is an _embedding_ bug, not a general Mozilla bug.  It may
work in Mozilla but PPEmbed, the Mac embedding test app, definitely has a problem.

Comment 9

[Conrad Carlen (not reading bugmail)]

Attachment: actual diff (don't look)

Patch sets the referrer, cleans up the AE handling (gets rid of assertion about
creating nsDependentCString with non-null-terminated string), and fixes allows
opening in new window from an image link. Also, CBrowserApp.cp was de-tabbed.

Comment 10

[Conrad Carlen (not reading bugmail)]

Attachment: -w diff for review

Same as other patch but with -w because of de-tabbed file.

Need r=/sr=.

Comment 11

[Simon Fraser [no longer active]]

+                    while (curr)
+                    {
+                        nsCOMPtr<nsIDOMElement> content = do_QueryInterface(curr);
+                        if (!content)
+                            break;
+                        linkElement = do_QueryInterface(content);
+                        if (linkElement)
+                            break;

Do you really need to QI twice?

+                CopyUCS2toASCII(temp, urlSpec);
...
+                if (NS_SUCCEEDED(rv))
+                    CopyUCS2toASCII(temp, referrer);

Hrm, URLs should be utf-8. You could either sent Unicode in the Apple Events, or
 send UTF-8.

Fix those, and sr=sfraser

Comment 12

[Steve Dagley]

Comment on attachment 92648 [details] [diff] [review]
-w diff for review

r=sdagley (what sfraser said)

Comment 13

[Conrad Carlen (not reading bugmail)]

Attachment: v2 actual diff (don't look)

Comment 14

[Conrad Carlen (not reading bugmail)]

Attachment: v2 -w diff for review

New patch uses utf-8 rather than ASCII in the urls passed though nsACStrings
and the AppleEvent. I didn't change the logic of that loop because I think 2
QIs are needed: the 1st just to see if the parent element is actually content
and the 2nd to see if it's a link. This can happen is the image's immediate
parent did not contain the href.

Comment 15

[Simon Fraser [no longer active]]

Comment on attachment 93178 [details] [diff] [review]
v2 -w diff for review

sr=sfraser

Comment 16

[Conrad Carlen (not reading bugmail)]

Comment on attachment 93178 [details] [diff] [review]
v2 -w diff for review

Forwarding sdagley's r= onto this one.

Comment 17

[Asa Dotzler [:asa]]

Comment on attachment 93178 [details] [diff] [review]
v2 -w diff for review

a=asa (on behalf of drivers) for checkin to 1.1 and 1.0.1

Comment 18

[Steve Dagley]

Conrad, ADT canceled their late meeting today so I didn't get to lobby for
approval but since it's critical for Tmumble please go ahead and check it in and
I'll deal with ADT.

Comment 19

[Conrad Carlen (not reading bugmail)]

Fixed on branch.

Comment 20

[Conrad Carlen (not reading bugmail)]

Fixed on trunk.

Comment 21

[Jaime Rodriguez, Jr.]

mdunn: pls verify this as fixed on the 1.0 branch, then replace the "fixed1.0.1"
keyword, with "verified1.0.1". thanks!

Comment 22

[Michael Dunn]

to Dharma for verification