wysiwyg: urls - pages created by document.write not added to session history

RESOLVED FIXED in mozilla0.9.9



Document Navigation
18 years ago
10 years ago


(Reporter: chriss, Assigned: Radha on family leave (not reading bugmail))



Dependency tree / graph

Firefox Tracking Flags

(Not tracked)


(Whiteboard: fix in hand, URL)


(1 attachment, 5 obsolete attachments)



18 years ago

DATE: 04/06/00

Web Integration Services personnel (Leyla Oswald, Jim Garza, Josh Metz) have 
detected the following bug with the new Netscape 6.0 browser and how it 
navigates to Merchant web sites from the Shop@ channels.

Across all Shop@Brands (AOL, AOL.com, Netcenter, CompuServe, Digital Cities), 
the Shop@ pages hosted by AOL load with no problem.  

When a Partner web site is called, the browser displays a blank page and does 
not load the called web page.  Hitting the BACK button sends the user two 
pages back instead of the previous page.

The symptoms noted in this email were found on both the Windows and MAC 
versions of Netscape 6.0 with the TYPICAL installation chosen.


1.  Starting from:

2.  Go to:

3.  Choose a Merchant Partner promotion such as the GAP. 

4.  Once selected, the new page does not load. 

5.  If you select BACK, you are directed to the main shopping page, instead 
of the previous Apparel page.

6.  This can be recreated consistently through the Shop@ department and 
Commerce Center pages across all Brands.


There are some anomalies to this recreation test:

-  Infrequently, after selecting a site and getting the blank screen, when 
going back you return to the correct shopping page.

- It also appears that the sites that are working seem to be Silver 
placements for the most part.  Silver placements can be found as hyperlinks 
at the bottom of Department level pages (e.g. 

- From all brands, links to Partner web sites from the Chic Simple catalogue 
mple/feature/index.adp) function properly.

Comment 1

18 years ago
adding beta 2 keyword fpr PDT review
Keywords: beta2

Comment 2

18 years ago
chriss@netscape.com have you tested this with current Mozilla nightly builds?

Comment 3

18 years ago
Added self to cc list.

Comment 4

18 years ago
with the Sunday tip, I get a page that says to use Netscape or IE when I click 
on a partner link.
When I click on the back button, I don't go back to the originating page.

Comment 5

18 years ago
-> Networking 
Assignee: asadotzler → gagan
Component: Browser-General → Networking
QA Contact: jelwell → tever

Comment 6

18 years ago
Sounds like a bug in the history mechanism. => radha
Assignee: gagan → radha


18 years ago
Keywords: nsbeta2

Comment 7

18 years ago
Whiteboard: [nsbeta2+]


18 years ago
Target Milestone: --- → M17

Comment 8

18 years ago
Move to M18 target milestone.
Target Milestone: M17 → M18

Comment 9

18 years ago
I need help reproducing this, sent mail to chriss.
Chris, thanks for helping me out.
Assignee: radha → mcafee

Comment 11

18 years ago
I just tried this with today's build, and though I was able to see the page
clicked to from the apparel page, I crashed going back.  Here is the talkback


Comment 12

18 years ago
oops. Removed cc incorrectly.

Comment 13

18 years ago
Adding Crash keyword.  I just tried it again and verified that I do indeed crash
using 2000071008
Keywords: crash

Comment 14

18 years ago
adding topcrash keyword.  this crash has made it to the top10 talkback crash 
list today.  here is some info, probably michael's entries:

nsGenericElement::HandleDOMEvent ca67d1e5
ment.cpp line 1370
        Build: 2000071009 CrashDate: 2000-07-10 UptimeMinutes: 2  Total: 2 
        OS: Windows NT  4.0 build 1381
        Comment: testing bug 35340 (going back from gap to apparel

    nsGenericElement::HandleDOMEvent ca67d1e5
ment.cpp line 1370
        Build: 2000071009 CrashDate: 2000-07-10 UptimeMinutes: 7  Total: 10 
        OS: Windows NT  4.0 build 1381
        Comment: Yep

Keywords: topcrash
Summary: Partner Shop@ pages not loading properly → Partner Shop@ pages not loading properly [@ nsGenericElement::HandleDOMEvent]

Comment 15

18 years ago
Using 2000071220 on Win98 I can no longer get this to crash, although I'm still
seeing the skip-a-page back behavior.  I'm following the steps outlined in the
bug.  I go to the Shop@ pages.  I click on the "Apparel" link in the upper left
of the page.  I click on the Gap ad in the page that comes up.  When the Gap
page loads, I click back.  It was at this point that I was crashing before,
though I don't see that problem now.  Now, instead of going back to the Apparel
page, I'm taken back to the original Shop@ page.

I don't see this as a huge problem any more, and I wouldn't recommend holding
beta for it.

Removing crash keyword.  Chris, if you agree with my beta assesment, could you
go ahead and change this to nsbeta2- in the status whiteboard?

Keywords: crash, topcrash

Comment 16

18 years ago
yes, I see the page skip now.

Comment 17

18 years ago
agree with michaell, nsbeta2-, over to radha
Assignee: mcafee → radha
Keywords: nsbeta2
Whiteboard: [nsbeta2+] → [nsbeta2-]

Comment 18

18 years ago
nav triage team:
need to fix redirecting
Whiteboard: [nsbeta2-] → [nsbeta2-][nsbeta3+]
Blocks: 47238
This page is very unique. Even 4.7 misbehaves on back and forward in this page. 

Here's what's going on:
1) After going to the second page(apparel or accessories section)  you click on 
a partner promotion page. At this time, the main content area replaces itself 
with 2 frames. This is *not* equivalent to going to a new frame set page. No 
loadURI() is called on the toplevel main content area. So, this screws up the 
normal assumptions docshell makes about loading pages with frameset. Because of 
this, the promotion partner page(GAP or ashford) does not get added to SH 
(though it is loaded). when you click back, SH just takes you to the page 1) 
because as for as SH is concerned, page 2) is the current page.

4.7 skips page 3)(the partner's site)  while going back and forward instead of 
skipping page 2)
Component: Networking → History
Adding nsbeta3 keyword to bugs which already have nsbeta3 status markings so 
the queries don't get all screwed up.
Keywords: nsbeta3
Adding nsbeta2 keyword to bugs with nsbeta2 triage value in status field so the 
queries don't get screwed up
Keywords: nsbeta2
This page is really complex. 4.7 also misbehaves in this page. I'm working on a 
fix for it. But the fix will bring seamonkey's behavior similar to 4.7's 
misbehavior. I do not believe this can be completely fixed. Waiting for rpotts 
to provide more feedback.
Whiteboard: [nsbeta2-][nsbeta3+] → [nsbeta2-][nsbeta3+]Waiting for rpotts's input
Priority: P3 → P2
*** Bug 46701 has been marked as a duplicate of this bug. ***
Discussed this withe rick today. This needs more work in layout and probably 
cache area. Rick and Nishseeth have set up a meeting to discuss this with 
Brendan and vidur. Details to emerge later
Whiteboard: [nsbeta2-][nsbeta3+]Waiting for rpotts's input → [nsbeta2-][nsbeta3+]Needs work in layout
Blocks: 46828

Comment 25

18 years ago
PDT marking P5 since page misbehaves in 4.7 and no progress in three weeks
Priority: P2 → P5
Whiteboard: [nsbeta2-][nsbeta3+]Needs work in layout → [nsbeta2-][nsbeta3+][PDTP5] Needs work in layout
Eric, this is the bug related to wyciwyg:// protocol. I'm assigning this to you, 
since you will be doing most of the work.
Assignee: radha → pollmann

Comment 27

18 years ago
The implementation is now done, testing begins...

Comment 28

18 years ago
Due to the P5 rating of this bug and a lack of time, I'm not going to get this
in for beta3.  If anyone would like to pick up where I left off, I'm going to
attach my work (based on the framework Gagan was kind enough to provide)

The file is a gzip'd tar, and the patch can be applied like so:
cd /builds/$USER/mozilla
tar -zxvf wyciwyg.tgz
patch -p0 < layout.patch
patch -p0 < netwerk.patch

On Windows, winzip should be happy to explode the tgz for ya.

The remaining problems are:
1) I used synchronous write methods in my implementation of nsWyciwygChannel. 
As Gagan predicted, they are a little sketchy.  For example, when someone loads
this document:
 <body onload="document.open();

The wyciwyg cache file (for some reason it's going to disk) contains this:

I also tried doing a flush after every write in
nsHTMLDocument::ScriptWriteCommon - no dice.

2) Reading the file from cache doesn't work (not investigated)

3) The session history drop-down entry is showing as "wyciwyg://0/http://..."
instead of the original URL.  Guessing this has something to do with session
history not checking the original URL, but I really don't know - this seems

4) The entry in session history for the original document is also showing up as
the wyciwyg URL.  I am probably doing something wrong in
nsHTMLDocument::OpenCommon - not investigated.
Keywords: helpwanted
Whiteboard: [nsbeta2-][nsbeta3+][PDTP5] Needs work in layout → [nsbeta2-][nsbeta3+][PDTP5] Needs debugging

Comment 29

18 years ago
Created attachment 14554 [details]
wyciwyg.tar.gz - first pass implementation

Comment 30

18 years ago
Created attachment 14556 [details]
same as above (with diff -u for readability)

Comment 31

18 years ago
Removing beta3+ - I won't be able to get to this one in time.
Whiteboard: [nsbeta2-][nsbeta3+][PDTP5] Needs debugging → [nsbeta2-][PDTP5] Needs debugging
Marking nsbeta3-.
Whiteboard: [nsbeta2-][PDTP5] Needs debugging → [nsbeta2-][nsbeta3-][PDTP5] Needs debugging
This bug is closely related (or possibly identical to) bug 46828.  This (or at
least the "skips one going back" behavior it shares with 46828) shows up in a
LOT of pages that work correctly for ns4.7.

I realize that Eric can't make it for nsbeta3.  Perhaps someone else can pitch
in?  In any case, due to the frequency this happens, and the problems it causes
(and that a dependant bug is marked nsbeta3 and severity critical w/ 7 votes)
I'm upping the severity on this one.  I would suggest that it also be upped from
a P5 to get it on more people's radar, but I won't touch it myself.  OS -> All,
Platform -> All
Severity: normal → critical
OS: Windows 98 → All
Hardware: PC → All

Comment 34

18 years ago
cc'ing chrisn, greggl - guys, this is not getting fixed on the client side so
the server side folks at Netscape Shopping better fix it on their side or push
back on client.
Adding RTM
This is really a bug in the browser - getting one site (netscape shopping) to
fix it isn't going to help us much.
We MUST address this for RTM (IMHO, of course).
This is a lot higher priority than 5.  See bug 46828 for examples - this breaks
all sorts of framed sites (most framed sites).
Keywords: rtm

Comment 36

18 years ago
adding keywords to (hopefully) improve findability of this bug.
Summary: Partner Shop@ pages not loading properly [@ nsGenericElement::HandleDOMEvent] → wysiwyg: wyciwyg: Partner Shop@ pages not loading properly [@ nsGenericElement::HandleDOMEvent]
*** Bug 54482 has been marked as a duplicate of this bug. ***
resummarising for better tracking. 
Summary: wysiwyg: wyciwyg: Partner Shop@ pages not loading properly [@ nsGenericElement::HandleDOMEvent] → wysiwyg: wyciwyg: urls not handled by history properly
Since this doesn't work properly on Mozilla or 4.7 it sounds like they have been
coding to an assumption about I.E. They really need to change the page so it's
friendly to Mozilla and 4.7. Based on this, marking it rtm-.
Whiteboard: [nsbeta2-][nsbeta3-][PDTP5] Needs debugging → [rtm-][nsbeta2-][nsbeta3-][PDTP5] Needs debugging
I should note that other bugs that depend on this one work perfectly correctly
in ns4.7 and ie, and fail only in mozilla.  Not fixing this one will inhibit
working on those if they really are dependant.

The problem in the testcase that Eric posted is not an IE-ism, it's a real
problem with mozilla.
No longer blocks: 46828
Post RTM, we need to take care of this for the next release. Removing old
keywordsetc.. and upping priority.  Netcenter webmail seems to use this too.
Blocks: 53026
Severity: critical → major
Keywords: nsbeta2, nsbeta3, rtm
Priority: P5 → P1
Whiteboard: [rtm-][nsbeta2-][nsbeta3-][PDTP5] Needs debugging → Needs debugging
No longer blocks: 53026
Blocks: 60946

Comment 42

18 years ago
Made some progress on this:

a) Merged to tip (changed PROGID to CONTRACTID in a few places)
b) Update to makefile.win's so it builds on Windows
c) Fixed the problems 1), 2), and 4) that I listed above

This leaves the following problems:
3) The string listed in the session history drop-down is a wyciwyg: url, not the
page title, the original url, or something descriptive like "(dynamic page)"
5) The fix does not work if the content that is written out is long, and
document.close() is not called.  I think this is because the cache entry is
still marked as "update in progress".  Perhaps going to a new URL should unmark
the entry we are leaving?

Tested, and this fixes Bug 46701
Should also fix Bug 54482
Does not fix the Netcenter URL above because it does not call document.close().

I'll attach the latest patch after some cleanup later...

Comment 43

18 years ago
updating milestone, keywords, cc, summary, status...
Keywords: helpwanted
Summary: wysiwyg: wyciwyg: urls not handled by history properly → wysiwyg: urls - pages created by document.write not added to session history
Whiteboard: Needs debugging → partial fix in hand
Target Milestone: M18 → M19

Comment 44

18 years ago
Fixing my milestone faux pas...  Thanks Eugene.  :)
Target Milestone: M19 → mozilla0.8
nav triage team: nsbeta1+ for checking in fix to 1,2,4. Nice to have 3 but
better to at least have 1,2,3 and track 4 separately. 
Keywords: nsbeta1


17 years ago
Target Milestone: mozilla0.8 → mozilla0.9
eric, how's this going - will we have it for mozilla0.9? thanks, Vishy
Eric: I think the third problem is because in nsISHEntry::GetTitle() if the
title for a page is empty, the URI is used a title.  THis change was made so
that directory listings, ftp urls etc.. won't show up empty in the menu.  I can
probably change that code to show something more decent for wyciwyg:// urls. If
the functionality is otehrwise working, can you checkin your chnages? 

Bugs in my list that depend on this include bug 58756 and 60946.


Comment 48

17 years ago
Radha, thanks - with a small change in the SH code you mention, I go the title
working now too - only have issue 5) to deal with - people who forget to
document.close() are not going to be happy (and there are lots of cases like
this on the web.  I still need to investigate the severity of 5) and possible
workarounds so that we can decide if checking this in with that issue is still
*** Bug 60946 has been marked as a duplicate of this bug. ***
*** Bug 58756 has been marked as a duplicate of this bug. ***

Comment 51

17 years ago
*** Bug 53026 has been marked as a duplicate of this bug. ***


17 years ago
Target Milestone: mozilla0.9 → mozilla0.9.1


17 years ago
Target Milestone: mozilla0.9.1 → mozilla0.9.2


17 years ago
Target Milestone: mozilla0.9.2 → mozilla0.9.3


17 years ago
Target Milestone: mozilla0.9.3 → mozilla0.9.4

Comment 52

17 years ago
eric is no longer around.  
we need some more time to figure out who can own this bug.
anybody have ideas?

unlikley this will make 0.9.4
Target Milestone: mozilla0.9.4 → mozilla0.9.5
We need to find a home for this one.  This is P1 Major with a partial fix
bit-rotting...  Reassigning this bug to Asa for reassignment, since no one has
stepped up to take it.
Assignee: pollmann → asa

Comment 54

17 years ago
radha, maybe? I'm the worst possible owner for this since I'm not even a developer.
Assignee: asa → radha
I'll see what I can do. May be for 1.0, as I'm not very familiar with the layout
Target Milestone: mozilla0.9.5 → mozilla1.0
testcase from bug 96215:


<script type="text/javascript">
function secondpage()
  document.write("<body>Second Page</body>");

<input type="button"  onclick="secondpage()" value="Go to Second Page">

This page isn't stored in session history when "Go to Second Page"
button is clicked.

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

Comment 58

17 years ago
the testcase isn't valid.
</body>"); should be executed by the html handler which causes the script to 
generate errors and fail to behave as intended.
iirc this doesn't do what authors think it does.
alternative suggestions include using "+" or a regexp


<script type="application/x-javascript">
function secondpage()
  var ohtml=
"<html><head><title>PAGE2<#title><#head><body>Second Page<#body><#html>";

<input type="button"  onclick="secondpage()" value="Go to Second Page">


i've stuck the testcase into the url field. fwiw old urls can be found by 
clicking the bug activity link

amusingly, i just triggered
Error: uncaught exception: Permission denied to call method HTMLDocument.write

I suppose to use my testcase i'd have to disable that exception...


17 years ago
Blocks: 104166
Keywords: mozilla1.0
Target Milestone: mozilla1.0 → mozilla0.9.8
I have been working on this for a while and have implemented the required
feature. I have tested my code with some of the testcases attached to this bug.
I would like a real world site probably off of netcenter to test my code. Can
some one point me to one?
Created attachment 65446 [details]
Patch to  implement the feature. windows .zip file
Attachment #14554 - Attachment is obsolete: true
Attachment #14556 - Attachment is obsolete: true
Created attachment 65448 [details]
patch v 1.1. Missed a file in the previous patch
Attachment #65446 - Attachment is obsolete: true
Keywords: mozilla1.0
Whiteboard: partial fix in hand → fix in hand

Comment 62

16 years ago
qa -> claudius
QA Contact: tever → claudius
Depends on: 119085

Comment 63

16 years ago
some review comments:

1) nsWyciwygChannel seems to have mixed line breaks.  some with \n some with
\r\n.  i suspect this might cause problems when you try to land this patch.

2) looks like nsWyciwygChannel has a mCallbacks member variable that it does not
use.  you have made nsWyciwygChannel implement nsIInterfaceRequestor, and you
pass |this| as the cache transport's notification callbacks.  but,
nsWyciwygChannel:: GetInterface should call mCallbacks->GetInterface.  this is
done to enable status and progress messages when reading from the cache.  you
should implement nsIProgressEventSink, so you can properly forward the
nsWyciwygChannel object as the nsIRequest object in the nsIProgressEventSink
methods.  see nsHttpChannel for an example of this.

3) looks like you aren't using a consistent brace style.  you have 
   foo() {



please pick one and stick with it.

4) use NS_INIT_ISUPPORTS() instead of NS_INIT_REFCNT()

5) no reason for a nsWyciwygChannel::Create method.  the protocol handler could
just use |new| to instantiate a nsWyciwygChannel object.

6) nsWyciwygProtocolHandler::Create could be replaced with a
NS_GENERIC_FACTORY_CONSTRUCTOR(nsWyciwygProtocolHandler) line in nsNetModule2.cpp

7) also, i'm not sure that this protocol handler belongs in necko.  it seems
like something that is layout specific, and should probably live elsewhere.  i'm
not sure where though.  if it is to be included in necko, then i think necko2 is
more appropriate.

8) indentation in nsWyciwygChannel seems very inconsistent.  please be consistent.

9) in nsWyciwygChannel::OnStopRequest you capture rv's, but you don't do
anything with them... not that there is anything to do at that point.  but, you
should either add some NS_ASSERTION or not bother capturing the rv's.

10) this section of code should be condensed:

  if (aWriteAccess)
    rv = cacheSession->OpenCacheEntry(aCacheKey, nsICache::ACCESS_WRITE,   
                                      PR_FALSE, getter_AddRefs(mCacheEntry));
    rv = cacheSession->OpenCacheEntry(aCacheKey, nsICache::ACCESS_READ, 
                                      PR_FALSE, getter_AddRefs(mCacheEntry));

how about this instead:

  nsCacheAccessMode accessMode = (aWriteAccess ? nsICache::ACCESS_WRITE
                                               : nsICache::ACCESS_READ);
  rv = cacheSession->OpenCacheEntry(aCacheKey, accessMode, PR_FALSE,

or how about changing the second parameter to OpenCacheEntry from PRBool to

11) in the layout patch, you seem to be making an unnecessary buffer copy:

 +  script = ToNewCString(text);
 +  // Save the script in cache
 +  nsCOMPtr<nsIWyciwygChannel> wcwgChannel(
 +  if (wcwgChannel)
 +    wcwgChannel->WriteToCache(script);
 +  nsCRT::free(script);
seems like |text.get()| could be used in place of |script| thereby eliminating a
strdup of the data.

anyways, you should make sure and get someone more familiar with the related
docshell and layout code to review those portions.  the necko code is for the
most part OK.  attach another patch, and i'll give it another review.

Comment 64

16 years ago
just a comment about a home for this handler. I agree that it probably shouldn't
live in necko (it has nothing to do w/ the networking layer). somewhere in
layout does make more sense. 
Created attachment 66319 [details]
Patch v 1.2 

This patch incorporates suggestions made  by darin and nisheeth.
Attachment #65448 - Attachment is obsolete: true
Created attachment 66324 [details]
v 1.3 Missed a file in the previous patch
Attachment #66319 - Attachment is obsolete: true
sr=jst for the content part, but we'll eventually need to eliminate the
NS_ConvertUCS2toUTF8(text).get() code in nsHTMLDocument::WriteCommon() for
perfornace reasons. There's already a bug on file for doing that (bug 121943).
Target Milestone: mozilla0.9.8 → mozilla0.9.9

Comment 68

16 years ago
some nits:

1- remove extra newline near bottom of nsIWyciwygChannel.idl
2- remove extra newline near bottom of nsWyciwygChannel.h
3- nsWyciwygChannel::mStatus is misaligned in nsWyciwygChannel.h
4- mLoadAttributes is unused.
5- indentation is not entirely consistent in nsWyciwygChannel.cpp -- please
review it carefully to make sure it is consistent.


5- if your channel is canceled or fails, you should Doom() the cache entry, but
it looks like you are not doing that.  with the current patch it would be
possible to end up with a partial cache entry, which i think you're code will
not know how to deal with.  you should probably add a nsresult parameter to
CloseCacheEntry and then if NS_FAILED(that nsresult) mCacheEntry->Doom().  see
nsHttpChannel::CloseCacheEntry as an example.

sorry for not catching (5) earlier.

Both NS4.x and IE manage the throbber well. ie., the throbber doesn't keep
running when there is no document.close(). When reload is hit, IE reloads the
page properly. I do not know if it hits the server. I will probably put my
testcase in a webserver and monitor the server logs. 

But NS 4.x has a mixed behavior. If the page has a document.close() reload on
that page works  properly. If the page does not have a document.write(), it
reloads the previous page.  
Fix checked into the trunk. 
Last Resolved: 16 years ago
Resolution: --- → FIXED

Comment 71

16 years ago
Two functions in the newly checked in nsWyciwygChannel.cpp can return
uninitialized nsresult values:

 `nsresult rv' might be used uninitialized in this function

 `nsresult rv' might be used uninitialized in this function

P.S. Off-topic: there already existed one such problem in content/html/document:

 `nsresult rv' might be used uninitialized in this function

and now there are three of them.

Resolution: FIXED → ---
Please do not modify an existing bug for a different problem. Open a new bug.
Bug manipulation confuses everyone who looks through it. 
Last Resolved: 16 years ago16 years ago
Resolution: --- → FIXED

Comment 73

16 years ago
Does it really make sense to file a new bug just for a trivial two-line fix to a
code that was just commited a few hours ago as a part of this one???

Comment 74

16 years ago
I've filed bug 122904 on the "may be used uninitialized" issue.

BTW, some info from that bug may also be worth mentioning here:

There is something else (besides the warning) wrong with
nsWyciwygChannel::WriteToCache, the code there is

if (!mCacheTransport && !mCacheOutputStream) {

    if (mCacheTransport)

Obviously, the second if statement is dead code.

Comment 75

16 years ago
Added nsbeta1 and topembed due to Bugscape 9169
Keywords: nsbeta1, topembed


10 years ago
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.