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)
Tracking
()
RESOLVED
FIXED
mozilla1.8alpha1
People
(Reporter: bugzilla, Assigned: bzbarsky)
References
()
Details
(Keywords: dom0)
Attachments
(2 files)
2.35 KB,
text/html
|
Details | |
7.87 KB,
patch
|
jst
:
review+
jst
:
superreview+
|
Details | Diff | Splinter Review |
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•22 years ago
|
||
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
Comment 2•22 years ago
|
||
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•22 years ago
|
||
jst, Should we support this at all?
Comment 4•22 years ago
|
||
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.
Updated•22 years ago
|
Target Milestone: --- → Future
Comment 5•21 years ago
|
||
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•21 years ago
|
||
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.
Assignee | ||
Comment 8•20 years ago
|
||
Can someone attach a testcase to this bug, please?
Comment 9•20 years ago
|
||
(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.
Assignee | ||
Comment 10•20 years ago
|
||
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•20 years ago
|
||
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.
Assignee | ||
Comment 12•20 years ago
|
||
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.
Assignee | ||
Updated•20 years ago
|
Attachment #147176 -
Flags: superreview?(jst)
Attachment #147176 -
Flags: review?(jst)
Comment 13•20 years ago
|
||
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+
Assignee | ||
Comment 14•20 years ago
|
||
> 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
Assignee | ||
Comment 15•20 years ago
|
||
And checked in to the 1.8a trunk.
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 16•20 years ago
|
||
*** Bug 250985 has been marked as a duplicate of this bug. ***
Comment 17•19 years ago
|
||
*** Bug 293409 has been marked as a duplicate of this bug. ***
Component: History: Session → Document Navigation
QA Contact: claudius → docshell
Comment hidden (offtopic) |
You need to log in
before you can comment on or make changes to this bug.
Description
•