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

Status: RESOLVED FIXED

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

Description

[tarquin]

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

Comment 1

[Tuukka Tolvanen (sp3000)]

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

Comment 2

[Tuukka Tolvanen (sp3000)]

btw looking at the w3.org docs, document.open() takes no parameters:
http://www.w3.org/TR/REC-DOM-Level-1/level-one-html.html#ID-72161170
http://www.w3.org/TR/DOM-Level-2-HTML/html.html#ID-72161170

Comment 3

[Radha on family leave (not reading bugmail)]

jst, Should we support this at all? 

Comment 4

[Johnny Stenback (:jst)]

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.

Comment 5

[Michael Sinz]

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.

Comment 6

[Michael Sinz]

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

Comment 7

[tarquin]

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.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Can someone attach a testcase to this bug, please?

Comment 9

[Michael Sinz]

Attachment: iframe document.open with replace option bug

(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.

Comment 10

[Boris Zbarsky [:bzbarsky]]

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....

Comment 11

[Johnny Stenback (:jst)]

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.

Comment 12

[Boris Zbarsky [:bzbarsky]]

Attachment: Patch

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.

Comment 13

[Johnny Stenback (: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!

Comment 14

[Boris Zbarsky [:bzbarsky]]

> Would this not be less code if you'd just have one call to SetLoadType()

Fixed that.  Good catch.

Comment 15

[Boris Zbarsky [:bzbarsky]]

And checked in to the 1.8a trunk.

Comment 16

[Boris Zbarsky [:bzbarsky]]

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

Comment 17

[Matthias Versen [:Matti]]

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

Comment 18

[Sunny Khan]

I am also learning to java script and figured out that the replace parameter is not working. For that reason i have researched a lot and found that this parameter works only in Internet Explorer. Javascript is worse language ever....