55558 - Infinite recursion when viewing help file [@ basic_nsAWritableString<unsigned short>::do_AppendFromReadable]

Status: RESOLVED INCOMPLETE

(Core :: DOM: Core & HTML, defect, P1)

Description

[Stephen P. Morse]

From menu select tasks->privacy->understanding-privacy.  You will get a help 
document.  Notice that there are links for "security center", "understanding 
security", and "privacy tutorial".  If you click on the privacy tutorial link 
you will hang the browser.  If you click on either of the other two links you 
will get a crash.

The crash is the result of an infinite recursion.  Stack trace (at least the end 
of the trace) is shown below.

Note that nsDocShell.cpp is involved in the resursion.  Note also that nisheeth 
checked in a fix to nsDocShell on Sept 27 to fix an infinite loop.  I wonder if 
this could be related.  Therefore assigning to nisheeth.

By the way, although I'm sure this problem exists in the mozilla branch as well 
as the netscape branch, you need to be running the netscape branch in order to 
demonstrate it.  Reason is that there is a different help file for the two 
branches.

-------

nsCharSourceTraits<nsReadingIterator<unsigned short> >::readable_distance(const 
nsReadingIterator<unsigned short> & {...}, const nsReadingIterator<unsigned 
short> & {...}) line 489 + 8 bytes
copy_string(nsReadingIterator<unsigned short> & {...}, const 
nsReadingIterator<unsigned short> & {...}, nsWritingIterator<unsigned short> & 
{...}) line 76 + 13 bytes
basic_nsAWritableString<unsigned short>::do_AppendFromReadable(const 
basic_nsAReadableString<unsigned short> & {...}) line 657 + 55 bytes
basic_nsAWritableString<unsigned short>::do_AppendFromElementPtr(const unsigned 
short * 0x0012f7cc) line 664 + 24 bytes
basic_nsAWritableString<unsigned short>::Append(const unsigned short * 
0x0012f7cc) line 401 + 28 bytes
nsAutoString::nsAutoString(const unsigned short * 0x0012f7cc) line 1780
nsDocShell::FindChildWithName(nsDocShell * const 0x04110a18, const unsigned 
short * 0x0012f7cc, int 1, int 1, nsIDocShellTreeItem * 0x00000000, 
nsIDocShellTreeItem * * 0x0012f850) line 1085
nsDocShell::FindItemWithName(nsDocShell * const 0x04110a14, const unsigned short 
* 0x0012f7cc, nsISupports * 0x03f096f0, nsIDocShellTreeItem * * 0x0012f850) line 
838 + 37 bytes
nsContentTreeOwner::FindItemWithName(nsContentTreeOwner * const 0x03f096f0, 
const unsigned short * 0x0012f7cc, nsIDocShellTreeItem * 0x03f09b84, 
nsIDocShellTreeItem * * 0x0012f850) line 154
nsDocShell::FindItemWithName(nsDocShell * const 0x03f09b84, const unsigned short 
* 0x0012f7cc, nsISupports * 0x04110a14, nsIDocShellTreeItem * * 0x0012f850) line 
871 + 61 bytes
nsDocShell::FindItemWithName(nsDocShell * const 0x04110a14, const unsigned short 
* 0x0012f7cc, nsISupports * 0x03f096f0, nsIDocShellTreeItem * * 0x0012f850) line 
858 + 61 bytes
nsContentTreeOwner::FindItemWithName(nsContentTreeOwner * const 0x03f096f0, 
const unsigned short * 0x0012f7cc, nsIDocShellTreeItem * 0x03f09b84, 
nsIDocShellTreeItem * * 0x0012f850) line 154
nsDocShell::FindItemWithName(nsDocShell * const 0x03f09b84, const unsigned short 
* 0x0012f7cc, nsISupports * 0x04110a14, nsIDocShellTreeItem * * 0x0012f850) line 
871 + 61 bytes
nsDocShell::FindItemWithName(nsDocShell * const 0x04110a14, const unsigned short 
* 0x0012f7cc, nsISupports * 0x03f096f0, nsIDocShellTreeItem * * 0x0012f850) line 
858 + 61 bytes
nsContentTreeOwner::FindItemWithName(nsContentTreeOwner * const 0x03f096f0, 
const unsigned short * 0x0012f7cc, nsIDocShellTreeItem * 0x03f09b84, 
nsIDocShellTreeItem * * 0x0012f850) line 154
nsDocShell::FindItemWithName(nsDocShell * const 0x03f09b84, const unsigned short 
* 0x0012f7cc, nsISupports * 0x04110a14, nsIDocShellTreeItem * * 0x0012f850) line 
871 + 61 bytes
nsDocShell::FindItemWithName(nsDocShell * const 0x04110a14, const unsigned short 
* 0x0012f7cc, nsISupports * 0x03f096f0, nsIDocShellTreeItem * * 0x0012f850) line 
858 + 61 bytes
nsContentTreeOwner::FindItemWithName(nsContentTreeOwner * const 0x03f096f0, 
const unsigned short * 0x0012f7cc, nsIDocShellTreeItem * 0x03f09b84, 
nsIDocShellTreeItem * * 0x0012f850) line 154
nsDocShell::FindItemWithName(nsDocShell * const 0x03f09b84, const unsigned short 
* 0x0012f7cc, nsISupports * 0x04110a14, nsIDocShellTreeItem * * 0x0012f850) line 
871 + 61 bytes
nsDocShell::FindItemWithName(nsDocShell * const 0x04110a14, const unsigned short 
* 0x0012f7cc, nsISupports * 0x03f096f0, nsIDocShellTreeItem * * 0x0012f850) line 
858 + 61 bytes

  ... and it just keeps going!

Comment 1

[verah (gone)]

Copying rudman.

Note that the link to "privacy tutorial" has an error -- <href=""> (no URL!). I 
can easily fix that, but an error like this shouldn't cause the browser to hang.

The other two links are to legitimate URLS; if I type them into the Location bar 
there's no problem. They are:
http://home.netscape.com/security/basics/index.html 
http://home.netscape.com/security/basics/index.html?cp=sciln

In both cases I added a "target="alfie11" modifier to open a new browser window.

Comment 2

[verah (gone)]

More...

This exact same file is accessed through the Help menu:
1. Open the Help menu.
2. Choose Help Contents.
3. Choose Understanding Privacy.

If you access the file via the Help menu, the links to "Security Center" and 
"Understanding Security" work fine.

However, if you open the file via Tasks:Privacy and Security, and THEN open the 
Help menu and follow the above steps, clicking the links in the copy you got via 
the Help menu now causes a crash.

Note: The Help menu version opens up in an independent window (our online help 
chrome). Does that make the difference?

Comment 3

[verah (gone)]

Copying Ian and nominating for rmt+.

Comment 4

[Stephen P. Morse]

If you remove the "target="alfie11", the crash will go away.  But I suppose 
that won't give you the behavior that you want.

Comment 5

[verah (gone)]

I was just about to try that, wondering if setting a target was somehow 
incorrect. Is it bad HTML?

The reason we want to open a new window is that this file is displayed through 
the help chrome when users choose it from the Help menu. We don't want the links 
to the Netcenter pages (Security Center and Understanding Security) to open in 
the Help chrome. 

This approach has been working up until today.

Comment 6

[verah (gone)]

I checked in an updated "securitycont.html" file to the trunk and branch; this 
will fix the missing link to "privacy tutorial."

Comment 7

[Nisheeth Ranjan]

Marking [rtm need info]

Comment 8

[verah (gone)]

Verified with the 10/11 build that the link to "privacy tutorial" is now 
working. The other two links ("security center" and "understanding security") 
still cause a crash.

Comment 9

[Stephen P. Morse]

This one's pretty serious.  If nobody comes up with a safe patch that will stop 
the recursion in vera's tutorial, then here's a real simple one that solves the 
problem.  It restores my original tutorial which does not have any crashing 
links.

Index: navigator.properties
===================================================================
RCS file: /m/src/ns/xpfe/browser/resources/locale/en-US/navigator.properties,v
retrieving revision 1.19.4.1
diff -u -r1.19.4.1 navigator.properties
--- navigator.properties        2000/10/04 02:49:19     1.19.4.1
+++ navigator.properties        2000/10/15 15:18:01
@@ -46,8 +46,8 @@
 intl.charset.detector=
 intl.charsetmenu.browser.static=iso-8859-1
 intl.charsetmenu.mailedit=ISO-8859-1, ISO-8859-15, armscii-8, ISO-8859-4, ISO-8
859-14, ISO-8859-2, GB2312, Big5, KOI8-R, windows-1251, KOI8-U, ISO-8859-7, ISO-
2022-JP, EUC-KR, ISO-8859-10, ISO-8859-3, TIS-620, ISO-8859-9, UTF-8, VISCII
-wallet.TutorialFromPrefs=chrome://communicator/content/help/help_security.xul
-wallet.TutorialFromMenu=chrome://communicator/content/help/help_security.xul
+wallet.TutorialFromPrefs=chrome://communicator/content/wallet/privacy.xul
+wallet.TutorialFromMenu=chrome://communicator/locale/wallet/privacy.html
 wallet.Server=http://home.netscape.com/wallet/
 wallet.Samples=http://home.netscape.com/bookmark/6_0b1/

Comment 10

[verah (gone)]

The problem is that the Help files (including html, dtd, and xul) are being 
loading inside the browser chrome instead of the Help chrome. We saw this in 
other cases, and the infinite recursion (and crash) happened in those instances 
as well.

I will prepare an HTML-only version of my document. We can't return to Steve's 
document, unless we're prepared to have it go through the review cycle that mine 
went through (including the pass through legal).

Comment 11

[verah (gone)]

I'm attaching the document. This is a single, simple HTML file, with no 
xul-based table of contents. Instead, the table of contents is at the beginning 
of the document (after the second paragraph). This document has no stylesheet, 
and should be hooked up to the Tasks:Security and Privacy submenu, and the "More 
Information button in the Cookies preference dialog, so that it appears in the 
main browser window when it is invoked.

Cross referenced URLs will also appear in the main browser window (that is,I've 
removed the "target= " tags.

Care should be taken not to re-open bug 56048 in the process of fixing this bug.

I'm emailing this file to Steve Morse, also, because he will know how to hook it 
up to the menu and button. I, alas, don't know how to do that. Also, I don't 
know what the effect is, at this point, of adding another file to be installed 
on the user's local machine.

Perhaps Nisheeth should reassign this to Steve...

I will bring this to the PDT's attention this afternoon.

Comment 12

[verah (gone)]

Attachment: Problem-free "Understanding Privacy" document for Tasks menu

Comment 13

[Nisheeth Ranjan]

Thanks, Vera and Steve for stepping in with workarounds.  I'm sorry I couldn't
find the cycles to fix this bug in time.  Re-assigning to Steve so that he can
make the changes that Vera has suggested.

Comment 14

[Daniel Veditz [:dveditz]]

I assume changing the help file will affect l10n and thus need their buy in.

Steve, what's the rest of the patch that would be required to get Vera's new doc
into the build? Is there a different small fix that would not have l10n impact,
or is this the safest thing to do at this point?

Comment 15

[Stephen P. Morse]

I haven't worked up the patch yet, but it will involve the following:

1. Vera needs to decide on a final resting place for her document
2. I modify the patch above so that it points to that location
3. I modify the makefiles so that they copy her document into the location 
indicated in 1

And that location needs to be in a directory that get localized.  And l10n then 
needs to produce localized copies of the document.

And, finally, there's the objection of code (actually text) duplication.  That 
is, the identical document now exists in two places.  The negative of that is 
that as changes are made in the future to one of the documents, it will easily 
get out of synch with the other one.

I'll look into this and see if I can come up with a clean solution.

Comment 16

[Stephen P. Morse]

What I want to do here is find out why the existing document is doing infinite 
recursion.  It would be much better to fix that problem than to do this band-aid 
solution of duplicating the help file and localizing two identical files.  That 
what we thought that Nisheeth was going to do for us since that falls into his 
area.

Comment 17

[Stephen P. Morse]

This problem is not just about viewing the helpfile from the task menu and from 
the pref panel.  If you view it the way it was originally intended to be viewed 
(from help->help-contents->understanding-privacy) you have the same crash.  So 
this sheds a completely different light on the situation.

For one thing, there is no longer any reason for us to be talking about two 
different help files -- one that gets brought up from the task menu and pref 
panel, and the other that gets brought up from the help menu.  The problem 
exists in all three places and whatever we do to fix it in one place should be 
done for all.

Second, I'm again leaning toward my original suggestion (made on 10-6) where I 
said that removing the target=alfie11 would stop the crash.  So let me propose 
the very simple patch of replacing alfie11 with _blank.  This one should make 
everybody happy (vera will be happy because we will be bringing up her tutorial 
rather than mine, I'll be happy because we won't have file duplication, nisheeth 
will be happy because he won't have to fix the underlying bug just yet, and pdt 
will be happy because the fix is so trivial that is assured to be risk-free).

It should be pointed out that there still is a problem with target= that 
Nisheeth will have to address someday.  But at least vera's help file will not 
crash when a link is clicked.

Comment 18

[Stephen P. Morse]

Attachment: Simple patch replacing alfie11 with _blank

Comment 19

[rudman]

Steve, your statement that "If you view it the way it was originally intended to
be viewed (from help->help-contents->understanding-privacy) you have the same
crash" is not true beginnning with the 10/16 build. See Bugscape bug 2468 for
details. (This Bugzilla bug should really be in Bugscape.) So there already has
been a fix in at least one place. But the fix there is not relevant for the fix
for this bug, it seems. I'm not disagreeing with your fix across the board, if
that works best---but note that the fix/appropriate behavior have been
implemented for the Help menu case.

Comment 20

[Stephen P. Morse]

Bugscape bug 2468 has to do with being able to close the help document.  That 
has nothing to do with the current bug.  The current bug is about a crash that 
occurs when you click on certain links in the help document.

Furthermore, bug 2468 says that the fix was checked in on 10-13.  I am running 
with a tree that I pulled and built today (10-16) and am definitely still 
observing the crashes described here.

So I disagree with you -- this crash occurs no matter how you bring up the help 
document, including from the help menu as originally intended.

Comment 21

[rudman]

Steve, bug 2468 evolved into something other than closing the Help window...A
new bug should have been filed but wasn't. The problem in general was that help
chrome was being opened in the main browser window. In that circumstance,
clicking a link in the "embedded" help window would cause a crash. That problem
has been fixed.

Regardless, I have the 10/16 Windows NT build, and when I choose Understanding
Privacy from the Help menu and click the links in question, a new browser window
appears, and there is no crash. However, the crash happens as Vera indicated on
10/6, if you first go to the Tasks menu, and then you go to the Help menu. If
you're seeing the crash even when you first go to the Help menu, what platform
are you using?

Comment 22

[verah (gone)]

As long as I first choose the document from the Help menu->Welcome screen, I 
don't see the bug, using yesterday's build on Windows. If I first choose the 
document using the Tasks menu route, and then go to the Help menu, I do see the 
bug. (Same as Steve R. notes above.)

Regarding localization: This document would be a nearly-exact copy of the 
current "Understanding Privacy." The only differences would be as follows: The 
table of contents (already translated) would be moved to appear after the second 
paragraph, a new heading "Understanding Privacy" -- a string that's already been 
translated anyway) would be added, two HREFs would be slightly changed (target 
modifiers removed), and one new anchor would be added.

(BTW I agree with Steve M. that it's not a good thing to have two identical 
documents, but feel it is okay as an emergency measure. In other words, we'd fix 
it for the next version.)

Comment 23

[Stephen P. Morse]

I am definitely seeing the crash using a tree that I pulled and built yesterday.  
Running on a win32 box (winnt).

From menu select help->help-contents
Small window appears saying "Netscape 6 help contents"
From list at left click on "Understanding Privacy"
Full-screen window appears saying "Netscape 6 Help: Understand Privacy"
Third paragraph contains a link for "security center" 
   and for "understanding security"
Click on either link
CRASH!!!

With the latest patch that I have included in this bug report, the crash does 
not occur.  Patch also fixes the identical crash if this dialog was accessed via 
the pref panel or the task menu instead of from the help menu.

Comment 24

[Stephen P. Morse]

SteveR & Vera:

Whether or not the crash occurs from the help menu is a moot point.  After 
applying my patch, the crash does not occur in any of the three places from 
which the dialog can originate.  Do you have any objections to going with my 
two-line patch rather than creating a completely new file (which is unnecessary) 
and modifying the associated make files.

Comment 25

[verah (gone)]

Steve M. has confirmed that the behavior of the links is not changed by his 
patch, so I concur -- it's certainly preferable to creating a new file.

r=verah

Comment 26

[Daniel Veditz [:dveditz]]

Since this is in the ns tree, I get to sr=dveditz. Changing to rtm+ for Steve's 
two-line target change.

Comment 27

[msanz]

adding mcarlson to the copy list

Comment 28

[Jay Patel [:jay]]

Here is one instance of this crash from talkback (there are others)...just 
wanted to make sure it was the same crash as the one discussed here.  Adding 
topcrash keyword and [@ basic_nsAWritableString<unsigned 
short>::do_AppendFromReadable] for tracking purposes:

basic_nsAWritableString<unsigned short>::do_AppendFromReadable f19e2405
        ../../dist/include/nsAWritableString.h line 652
        Build: 2000100909 CrashDate: 2000-10-09 UptimeMinutes: 253  Total: 253 
        OS: Windows NT  4.0 build 1381
        URL: 
        Comment: Clicked on "Security Center" link on the Understanding Privacy 
page reachable through the Tasks --> Privacy -->Understand Privacy menu item.  
Crashed.
         Detailed : http://climate/reports/incidenttemplate.cfm?bbid=18822432
         StackTrace: 
http://climate/reports/stackcommentemail.cfm?dynamicBBID=18822432

Comment 29

[Peter Trudelle]

PDT says rtm++, please land on branch ASAP

Comment 30

[Stephen P. Morse]

The change to the helpfiles have now been checked in.  Removing rtm++ and 
reassigning to Nisheeth to fix the underlying problem.  In order to demonstrate 
the problem, you'll need to undo the patch and then follow the steps descibed 
here.

Comment 31

[Nisheeth Ranjan]

Marking rtm- to get off the rtm radar.

Comment 32

[Heikki Toivonen (remove -bugzilla when emailing directly)]

With the patch in, this is not a topcrash bug.  Removing the topcrash keyword so
that this bug doesn't show up in chofmann's weekly "Key Bug Metrics" report.

Comment 33

[Nisheeth Ranjan]

Moving to 0.9.1...

Comment 34

[Nisheeth Ranjan]

-> 0.9.2

Comment 35

[Blake Ross]

Doesn't look like this is getting fixed before the freeze tonight.
Pushing out a milestone.  Please correct if I'm mistaken.

Comment 36

[Nisheeth Ranjan]

Bulk re-assign of my 0.9.4 bugs to Heikki.  I will not have the cycles to work
on these bugs while Clayton is on sabbatical for the next six weeks.

Comment 37

[Brant Gurganus]

By the definitions on <http://bugzilla.mozilla.org/bug_status.html#severity> and
<http://bugzilla.mozilla.org/enter_bug.cgi?format=guided>, crashing and dataloss
bugs are of critical or possibly higher severity.  Only changing open bugs to
minimize unnecessary spam.  Keywords to trigger this would be crash, topcrash,
topcrash+, zt4newcrash, dataloss.

Comment 38

[Frank Wein [:mcsmurf]]

Anyone knows if this bug is still valid? I can't seem to find this document
anymore...

Comment 39

[Daniel Wang]

The file has been removed.
Though we might still have the bug with <a target="xxxx"> in chrome://

Comment 40

[Nickolay_Ponomarev]

No point in keeping a bug, that no-one can reproduce, open, just because the underlying issue might have not been fixed in the last 9 years. Closing as incomplete.