Last Comment Bug 178729 - opener.location replaces parent history.
: opener.location replaces parent history.
Product: Core
Classification: Components
Component: DOM: Core & HTML (show other bugs)
: Trunk
: All All
P2 normal (vote)
: mozilla1.8alpha2
Assigned To: Boris Zbarsky [:bz] (still a bit busy)
: Hixie (not reading bugmail)
: Andrew Overholt [:overholt]
Depends on:
  Show dependency treegraph
Reported: 2002-11-06 13:37 PST by Ian
Modified: 2004-06-01 09:17 PDT (History)
3 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Testcase for bug. (2.54 KB, text/html)
2002-11-06 13:39 PST, Ian
no flags Details
Patch to that effect. (1.26 KB, patch)
2004-05-31 22:46 PDT, Boris Zbarsky [:bz] (still a bit busy)
jst: review+
jst: superreview+
Details | Diff | Splinter Review

Description User image Ian 2002-11-06 13:37:34 PST
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.2b) Gecko/20021016
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.2b) Gecko/20021016

If a pop opens and emediatly set the opener.location variable then the parent
window will go to the location however if the back button is pressed on the
parent it does not go to the previous window (history(-1)) it goes to the next
one (history(-2))

Reproducible: Always

Steps to Reproduce:
See Attachement that will follow.

Actual Results:  
Goes to previous Previous window (history(-2))

Expected Results:  
Back button should go to previous window (history(-1))

I test this on the Todays nightly build and the bug was still there.
Comment 1 User image Ian 2002-11-06 13:39:04 PST
Created attachment 105376 [details]
Testcase for bug.
Comment 2 User image Asa Dotzler [:asa] 2004-03-23 16:36:51 PST
I don't see the parent window update here. Is this still a bug?
Comment 3 User image Ian 2004-03-24 04:43:22 PST
This bug still exist on build 2004031616 on windows 2000

The attachement supplied does not work from the bugzilla site as I thought it
would. This is probably due to the url parameters(I suspect). You have to see
this test as being 2 seperate pages - I simply coded it in one page because I
thought I could get it working in bugzilla (I guest not)

What you need to do is save the html page to your system then open it in mozilla.

For the first test hit "popup window with automatic parent update" then close
the popup. You should notice that if you hit the back button on your browser it
will not go anywheres or it will go the page that you we looking at prior to
looking at the testcase.

Now the second test will attemp to do the same thing as the first test except in
a manual way. Hit "pop Up Window" and then within the pop window hit "Update
Parent". Now you can close the pop up window. This time you will notice that if
you hit the back button on your browser it will take you back a page - to the
beginning of the test case.

Both of these test (I would think) should product the same results but they
don't seem to.
Comment 4 User image Jesiah S 2004-05-18 12:46:46 PDT
Adding a URL to a testcase from bug 240436.
Comment 5 User image Boris Zbarsky [:bz] (still a bit busy) 2004-05-31 22:34:33 PDT
Confirming.  This is a regression from bug 72197.

The issue is that that bug added a check for whether we're in a <script> tag
execution and does the equivalent of location.replace() if we are.  That screws
up history, of course.

The problem here is that the <script> is being executed in one window while the
location is being set in a different window.  It seems pretty clear that in that
case we should NOT be clobbering history like that.

On the other hand, what happens if a subframe runs a <script> that resets the
location of a parent of the subframe?  Should that be a replace load?  I'm
thinking "no", myself... (otherwise a subframe loading when you click on a link
in a frameset can wipe out the whole frameset _and_ make it impossible to go
back to it).

So I think we should compare the window that the location object belongs to with
the window the script is running in and only replace if they are the same.
Comment 6 User image Boris Zbarsky [:bz] (still a bit busy) 2004-05-31 22:46:10 PDT
Created attachment 149734 [details] [diff] [review]
Patch to that effect.
Comment 7 User image Boris Zbarsky [:bz] (still a bit busy) 2004-05-31 22:47:31 PDT
Comment on attachment 149734 [details] [diff] [review]
Patch to that effect.

jst, what do you think?  I checked that this does not regress bug 72197 or bug
Comment 8 User image Johnny Stenback (:jst, 2004-05-31 23:03:37 PDT
Comment on attachment 149734 [details] [diff] [review]
Patch to that effect.

+	     // Now check to make sure that the script is running in our
+	     // since we only want to replace if the location is set by a
+	     // <script> tag in the same window.  See bug 178729.
+	     nsCOMPtr<nsIScriptGlobalObject>
+	     inScriptTag = (ourGlobal == scriptContext->GetGlobalObject());

It'd be more correct to check if scriptContext == ourGlobal->GetContext(), but
both ways'll get you the same answer, so I'm cool either way (and I think the
way you wrote it saves you a null check, so that's cool).

Comment 9 User image Boris Zbarsky [:bz] (still a bit busy) 2004-06-01 09:17:57 PDT

Note You need to log in before you can comment on or make changes to this bug.