opener.location replaces parent history.

RESOLVED FIXED in mozilla1.8alpha2



16 years ago
15 years ago


(Reporter: donguana, Assigned: bzbarsky)



Firefox Tracking Flags

(Not tracked)




(2 attachments)



16 years ago
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

16 years ago
Created attachment 105376 [details]
Testcase for bug.

Comment 2

15 years ago
I don't see the parent window update here. Is this still a bug?

Comment 3

15 years ago
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

15 years ago
Adding a URL to a testcase from bug 240436.
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.
Assignee: radha → general
Component: History: Session → DOM: Level 0
Ever confirmed: true
OS: Windows 2000 → All
QA Contact: claudius → ian
Hardware: PC → All
Created attachment 149734 [details] [diff] [review]
Patch to that effect.
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
Attachment #149734 - Flags: superreview?(jst)
Attachment #149734 - Flags: review?(jst)
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).

Attachment #149734 - Flags: superreview?(jst)
Attachment #149734 - Flags: superreview+
Attachment #149734 - Flags: review?(jst)
Attachment #149734 - Flags: review+


15 years ago
Assignee: general → bzbarsky
Priority: -- → P2
Target Milestone: --- → mozilla1.8alpha2
Last Resolved: 15 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.