Closed Bug 164821 Opened 22 years ago Closed 21 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 Patch (deleted) — Splinter 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: 21 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: