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)
Tracking
()
RESOLVED
FIXED
mozilla1.8alpha1
People
(Reporter: bugzilla, Assigned: bzbarsky)
References
()
Details
(Keywords: dom0)
Attachments
(2 files)
(deleted),
text/html
|
Details | |
(deleted),
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•22 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•21 years ago
|
||
Can someone attach a testcase to this bug, please?
Comment 9•21 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•21 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•21 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•21 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•21 years ago
|
Attachment #147176 -
Flags: superreview?(jst)
Attachment #147176 -
Flags: review?(jst)
Comment 13•21 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•21 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•21 years ago
|
||
And checked in to the 1.8a trunk.
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 16•20 years ago
|
||
*** Bug 250985 has been marked as a duplicate of this bug. ***
Comment 17•20 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
•