Closed Bug 164821 Opened 22 years ago Closed 20 years ago

document.open('text/html','replace') does not replace history entry

Categories

(Core :: DOM: Navigation, defect, P2)

x86
All
defect

Tracking

()

RESOLVED FIXED
mozilla1.8alpha1

People

(Reporter: bugzilla, Assigned: bzbarsky)

References

()

Details

(Keywords: dom0)

Attachments

(2 files)

When using document.open('text/html','replace') it is supposed to replace the 
existing history entry but it does not. The URL gives my iframe scrollers. 
After they have scrolled a couple of messages, you will see that each new 
message has added an entry into the browsers history, even though I use 
document.open('text/html','replace')

All other browsers I have tested work fine and comply with the Netscape 
documentation on
http://developer.netscape.com/docs/manuals/communicator/jsref/doc1.htm#1011112
linux, m1.1: Confirm title of main page "(Simple) message scroller" (not of the
iframe content, "Dynamic Content") appears in session history over and over
again. ->history
Assignee: rogerl → radha
Status: UNCONFIRMED → NEW
Component: JavaScript Engine → History: Session
Ever confirmed: true
QA Contact: pschwartau → claudius
Keywords: dom0
jst, Should we support this at all? 
Yes, we should support this for backwards compatibility. It's easy enough to
look at the value of the second argument and do the right thing wrt session
history, assuming we have the API's avaliable to do the session history part.
Have a look at nsHTMLDocument::ScriptWriteCommon() for a sample on how to get
the value of the second JS argument.
Target Milestone: --- → Future
I have been running into the same problem with document.open(...,'replace') not
working correctly.  In my WordWiggle.com web site, the game play has your word
selection list in a JavaScript controlled IFrame.  Each time the IFrame is
re-written a fresh entry in the history is added.  This makes for an annoying
history list (since there is no way to clear it from JavaScript)

This seems to work correctly in all other browsers that I have tested with that
have working IFrames.

For an example, see http://www.wordwiggle.com/ and click on practice to run a
practice game.
BTW - I have worked around the need for an IFrame (albeit, this has broken the
site for use with Safari rather badly)  I have left up a simple test/bug page
that still shows the problem at http://www.wordwiggle.com/bug.html
In response to 'Should we support this at all?';
Yes. Because to my knowledge there is nothing else that performs the same 
function, and for that reason, I think the W3C was wrong not to include it in 
the specification (just like they were wrong not to include innerHTML, one of 
the few Microsoft extensions that are worth keeping).

The only workaround for this is (I think) to use:
window.frames['iframeName'].window.document.body.innerHTML
but this may not work in older browsers. I've not tested it in Safari etc. but 
it produces errors in IE4.

Not that it is really important in this report, but Opera is right not to write 
to about:blank, as about: does not match the protocol of the current page (http:
). Therefore, the browser's security model kicks in.

You need to use window.open('','iframeName'), but that also adds a history 
entry. Setting the initial src to '' will not work, as most browsers treat 
src="" as src="./". Annoying.
Can someone attach a testcase to this bug, please?
(In reply to comment #8)
> Can someone attach a testcase to this bug, please?

See comment #5 or go to http://www.wordwiggle.com/bug.html

This page happens to show various bugs in various browsers but it a rather
simple page.
jst, I tried adding the following code to nsHTMLDocument::Open(nsIDOMDocument**
aReturn) :

  nsCOMPtr<nsIXPConnect> xpc(do_GetService(nsIXPConnect::GetCID()));
  nsCOMPtr<nsIXPCNativeCallContext> ncc;
  if (xpc) {
    rv = xpc->GetCurrentNativeCallContext(getter_AddRefs(ncc));
    NS_ENSURE_SUCCESS(rv, rv);
  }
  fprintf(stderr, "ncc: %p\n", ncc.get());

I get "ncc: (nil)" on that testcase.  It looks like the problem is that the
actual call to nsIDOMNSHTMLDocument::open comes from
nsHTMLDocumentSH::DocumentOpen.  It's odd that that loses the native call
context, no?

In any case, if I can sort out how to get at the argv to the JS call I think I
know how to beat session history into submission....
That's because calls to document.open() don't go through XPConnect. See
http://lxr.mozilla.org/mozilla/source/dom/src/base/nsDOMClassInfo.cpp#5365 (as
you already did), and
http://lxr.mozilla.org/mozilla/source/dom/src/base/nsDOMClassInfo.cpp#5365 to
see what's going on there :-)

Feel free to add any argument handlign to nsHTMLDocumentSH::DocumentOpen()
(where it belongs) and change nsIDOMNSHTMLDocument (note the "NS") to take
whatever arguments that are needed here.
Attached patch PatchSplinter Review
I went with a case-sensitive comparison, since nothing I could find indicated
that a string like "rEPlAcE" was special as a second arg to document.open(). 
Note that the documentation at
http://msdn.microsoft.com/workshop/author/dhtml/reference/methods/open_1.asp is
not internally consistent, not consistent with the actual behavior of IE, and
not consistent with the implementation of document.open in NS4, so I pretty
much ignored it.
Attachment #147176 - Flags: superreview?(jst)
Attachment #147176 - Flags: review?(jst)
Comment on attachment 147176 [details] [diff] [review]
Patch

- In nsHTMLDocument::OpenCommon():

+    if (aReplace) {
+      docshell->SetLoadType(nsIDocShell::LOAD_CMD_NORMAL |
+			     (nsIWebNavigation::LOAD_FLAGS_REPLACE_HISTORY <<
16));
+    } else {
+      // Make sure that we're doing a normal load, not whatever type
+      // of load was previously done on this docshell.
+      docshell->SetLoadType(nsIDocShell::LOAD_CMD_NORMAL |
+			     (nsIWebNavigation::LOAD_FLAGS_NONE << 16));
+    }
+    

Would this not be less code if you'd just have one call to SetLoadType() and
just set up a local to hold the right value and pass that to the one call to
SetLoadType()?

r+sr=jst. Very nice!
Attachment #147176 - Flags: superreview?(jst)
Attachment #147176 - Flags: superreview+
Attachment #147176 - Flags: review?(jst)
Attachment #147176 - Flags: review+
> Would this not be less code if you'd just have one call to SetLoadType()

Fixed that.  Good catch.
Assignee: radha → bzbarsky
Priority: -- → P2
Target Milestone: Future → mozilla1.8alpha
And checked in to the 1.8a trunk.
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
*** Bug 250985 has been marked as a duplicate of this bug. ***
*** Bug 293409 has been marked as a duplicate of this bug. ***
Component: History: Session → Document Navigation
QA Contact: claudius → docshell
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: