125037 - crash on double-click of word in textfield after form submission - Trunk M1BR [@ CopyChars1To2] [@ nsStrPrivate::EnsureCapacity]

Status: RESOLVED WORKSFORME

(Core :: Layout, defect)

Description

[Michael Smith]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:0.9.8+) Gecko/20020212
BuildID:    2002021206

Mozilla crashes if you double click on a word in a search text field after
submitting the form, but before the page is replaced with the new page.  Double
clicking works to select the word up to the point that the title changes (with
tabbed browsing, I was noticing the tab changing).  Once that title changes, if
you double click before the page is replaced with the new page, it crashes.

Originally seen when I was looking up a word at http://www.m-w.com, but verified
the same thing happens when using the form at http://bugzilla.mozilla.org

Reproducible: Always
Steps to Reproduce:
1. Load a page with a form that has an text input field
(http://bugzilla.mozilla.org)
2. Type a word into the text field
3. submit form (reproduced easier if you use "enter" to submit the form and keep
the mouse hovering over the word you typed).
4. As soon as the title bar and/or tab title changes, but before the current
page is actually replaced, double click the word you typed into the text field.

Actual Results:  It crashed.

Expected Results:  It doesn't crash. 

I originally saw with 0.9.8, but upgraded to nightly (2002021206) to make sure
the problem is still there. Each crash generated a talkback thing.  Here's some
talkback IDs:

With 0.9.8:
 TB2822975Q
 TB2796544G
 TB2796510H
 TB2796494Z

With 2002021206:
 TB2825507K
 TB2825273H
 TB2824725H
 TB2824540Q
 TB2823370Y

Comment 1

[Boris Zbarsky [:bzbarsky]]

stephend, could you get a stack?

Comment 2

[[e-mail] (gone - use [e-mail] instead)]

CopyChars1To2()
nsStr::StrAppend()
nsString::AppendWithConversion()
nsString::AssignWithConversion()
nsPlainTextSerializer::AppendText()
nsDocumentEncoder::SerializeNodeStart()
nsDocumentEncoder::SerializeRangeToString()
nsDocumentEncoder::EncodeToString()
nsCopySupport::HTMLCopy()
nsAutoCopyService::NotifySelectionChanged()
nsTypedSelection::NotifySelectionListeners()
nsSelection::NotifySelectionListeners()
nsSelection::SetMouseDownState()
nsTextInputSelectionImpl::SetMouseDownState()
nsFrame::HandleRelease()
nsFrame::HandleEvent()
PresShell::HandleEventInternal()
PresShell::HandleEvent()
nsViewManager::HandleEvent()
nsView::HandleEvent()
nsViewManager::DispatchEvent()
HandleEvent()
nsWidget::DispatchEvent()
nsWidget::DispatchWindowEvent()
nsWidget::DispatchMouseEvent()
nsWidget::OnButtonReleaseSignal()
nsWindow::HandleGDKEvent()
dispatch_superwin_event()
handle_gdk_event()
libgdk-1.2.so.0 + 0x17d7f (0x40359d7f)
libglib-1.2.so.0 + 0x11773 (0x4038c773)
libglib-1.2.so.0 + 0x11d39 (0x4038cd39)
libglib-1.2.so.0 + 0x11eec (0x4038ceec)
libgtk-1.2.so.0 + 0x94333 (0x402a8333)
nsAppShell::Run()
nsAppShellService::Run()
main1()
main()
libc.so.6 + 0x1c507 (0x404d3507) 

Comment 3

[Boris Zbarsky [:bzbarsky]]

crash in string code...

But feel free to punt if you think the serializer's passing in a non-terminated
string or some such.  :)

Comment 4

[Andrew Schultz]

Attachment: stack trace with symbols

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:0.9.8+) Gecko/20020214
in the terminal, I get:
WARNING: NS_ENSURE_TRUE(globalObject) failed, file nsDocumentEncoder.cpp, line
216

Comment 5

[Boris Zbarsky [:bzbarsky]]

Ok.  Over to the document encoder then.  Sounds like it's not doing the right
thing.... Should it even be doing anything at all if there is no script global
object at that point??

Comment 6

[harishd]

Tanu: Could you please take a look at this? Thanx

Comment 7

[Tanu Mutreja]

Some observations: 
o On repeating the same steps mozilla hangs instead of a crash.
o It's not reproducible on WinNT. Bug also mentions it specifically for Linux.
(No conflict just to confirm).
o On Linux it hangs at the same point as mentioned in comment#4.
o On Windows, on repeating exactly the same steps it does not even reach to 
this point. So problem may not be at this point.

Queries:
o On submitting a form, is there something like the original form gets locked 
for actions like selecting text box etc. till it gets the response back? If not,
should it be like this ? Or should the form just be locked for any java script 
event to get triggered i.e. javascript is disabled kinda?
o Any idea if we should really reach to this point in nsDocumentEncoder?
o In nsDocumentEncoder, what is expected behavior if GetPrincipal
()/GetScriptGlobalObject() or GetContext() fail? CC'ing to jfrancis if he can 
help me on this. 

Comment 8

[Boris Zbarsky [:bzbarsky]]

From a DOM point of view, GetScriptGlobalObject() means the document is far
enough in teardown that things may have dangling pointers and the like. 
Basically, the DOM should really not be accessed if there is no script global
object....

Comment 9

[scottputterman]

Moving Netscape owned 0.9.9 and 1.0 bugs that don't have an nsbeta1, nsbeta1+,
topembed, topembed+, Mozilla0.9.9+ or Mozilla1.0+ keyword.  Please send any
questions or feedback about this to [e-mail].  You can search for
"Moving bugs not scheduled for a project" to quickly delete this bugmail.

Comment 10

[Tanu Mutreja]

On Linux, double clicking on text triggers copy event. I got exactly the same 
stack trace as attached here by Andrew Schultz (comment#4). Problem may be 
somewhere in the following part:

#9  0x41f9b5be in nsAutoCopyService::NotifySelectionChanged (this=0x82494d0, 
aDoc=0x86ad4ac, aSel=0x87ece38, aReason=4) 
at ../../../dist/include/xpcom/nsCOMPtr.h:643
#10 0x414249fe in nsTypedSelection::NotifySelectionListeners (this=0x87ece38) 
at ../../../dist/include/xpcom/nsCOMPtr.h:650
#11 0x41411bf9 in nsSelection::NotifySelectionListeners (this=0x87ecd70, 
aType=1) at nsSelection.cpp:3052

On viewing the values of member variables of nsTypedSelection object, I see 
that mStartOffset and mEndOffset are having negative values and those values 
are carried forward till nsDocumentEncoder->nsPlainTextSerializer, which 
probably does not expect a negative value (but does not do any error handling 
also) and it gets hung there.

I need some help in the layout part. CC'ing some related guys if they can 
provide some information. I need to know if the things are fine at this part of 
the code.

Comment 11

[Shiva Thirumazhusai]

Since it is a reproducible crash I am nominating it for nsbeta1 and Changing the 
milestone to Mozilla 1.0.

Comment 12

[Boris Zbarsky [:bzbarsky]]

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

Comment 13

[Tanu Mutreja]

Attachment: Fix for the Serializer Part...

Patch does not resolve the root cause of bug but it just helps in hiding this
crash. In fact caller should be stopped much before the serializer is invoked.

I'm working on the core part of the problem but irrespective of how we fix the
root problem, I feel that, applying this patch is something good to ensure
safety in the Serializer code.

Comment 14

[Daniel Bratell]

Comment on attachment 75178 [details] [diff] [review]
Fix for the Serializer Part...

Since the serializer is kind of a part of it self I guess it's ok to not trust
the caller too much. I would rather see a fix for the underlying cause though,
so I hope it won't end with this patchy patch. I mean, if we can't trust the
length, can we trust the pointer? What if the length is too much, that won't be
detected? How about the other serializers, do they crash too?

Anyway, r=bratell for making the serializer a little more robust.

Comment 15

[Johnny Stenback (:jst)]

Comment on attachment 75178 [details] [diff] [review]
Fix for the Serializer Part...

sr=jst

Comment 16

[Scott Collins]

Comment on attachment 75178 [details] [diff] [review]
Fix for the Serializer Part...

a=scc

Comment 17

[Vinay Badami]

Patch checked in
D:\mozilla\content\base\src>cvs commit
cvs commit: Examining .
Checking in nsPlainTextSerializer.cpp;
/cvsroot/mozilla/content/base/src/nsPlainTextSerializer.cpp,v  <-- 
nsPlainTextSerializer.c
new revision: 1.51; previous revision: 1.50
done

Comment 18

[Michael Smith]

Using the lastest nightly (build 2002032508), I tried verifying that the
checked-in patch avoids the crash, but am still able to reproduce the crash. 
Tried three times, and crashed each time.  Talkback IDs: TB4454969Z, TB4455090K,
and TB4455198M

Comment 19

[Tanu Mutreja]

Smith, with the latest code on my tree, I'm unable to reproduce the crash. Would  
you please verify that your mozilla/content/base/src/nsPlainTextSerializer.cpp 
is having the changes from this patch. I doubt about the branches we are using! 
Another thing I notice is that, your stack trace is having "Segmentation Fault", 
which was never the case on my tree. I feel the it's crashing due to a different 
reason but at the same place. It would be quite helpful if you can confirm that 
you are following the exact steps mentioned in this bug. 

Comment 20

[Michael Smith]

I'm not building mozilla from source (and don't have the time/resources to do
so), so I cannot verify that the mozilla I'm using has the actual change in it.
 I downloaded the latest nightly again, and tried again just in case the patch
really wasn't in 2002032508.  This time, I got version 2002032606, and am still
able to reproduce with the same procedure described in the original report.  

Talkback IDs from build 2002032606:
TB4492794Q
TB4492865K
TB4492883W

Comment 21

[Andrew Schultz]

I crash with Mozilla's 2002032608, but not with a CVS build from 20020326. 
options for the CVS build were were:
--disable-dtd-debug --enable-optimize=-O

perhaps the crash is only occuring with a non-debug build.

Comment 22

[Andrew Schultz]

no crash with non-debug CVS build either...

Comment 23

[Asa Dotzler [:asa]]

Comment on attachment 75178 [details] [diff] [review]
Fix for the Serializer Part...

obsoleting patch since it has landed but the bug is still open.

Comment 24

[Michael Smith]

I am still able to reliably reproduce this crash with latest nightly. 
Build 2002041006
Talkback IDs:
 TB5040567Z
 TB5040582Y
 TB5040600Y
 TB5040688K

I don't know what branch that patch supposedly went into, but it doesn't seem to
be working for me on the nightly builds downloaded from the mozilla.org front page:

http://ftp.mozilla.org/pub/mozilla/nightly/latest/mozilla-i686-pc-linux-gnu-installer.tar.gz

Comment 25

[Andrew Schultz]

anyone got any idea what build flags the nightly build scripts use?

Comment 26

[Michael Smith]

I just tried with 1.0 RC1, and am still able to reproduce the crash (talkback ID
TB5356283G).  

I also just noticed the target milestone of 1.0.1.  Is this not going to be
fixed in 1.0?  It's a very easily reproducible crash.  See comment 11.

Comment 27

[Tanu Mutreja]

Component -> layout.

Though the bug at present is fixed but should remain *OPEN* till we actually 
fix the layout part invloved in it.

Comment 28

[Michael Smith]

Um.  Where exactly is this bug fixed?  As I have mentioned, I can still 
reproduce it.  See comment 18, comment 20, comment 24, and comment 26.  

Comment 29

[Michael Smith]

Still not fixed in 1.0 RC2.
talkback ID: TB6270282M

Also of note: You do not necessarily need to "submit the form" as described in
the original report.  You can also reproduce this bug by having a form field
viewable on the screen, load a bookmark, and then double-clicking on a word in
the visible form before the bookmark is fully loaded: TB6270212G.

Comment 30

[Boris Zbarsky [:bzbarsky]]

stephend, would you get the stack? TB6270282M

Comment 31

[[e-mail] (gone - use [e-mail] instead)]

CopyChars1To2()
nsStrPrivate::StrAppend()
nsString::AppendWithConversion()
nsString::AssignWithConversion()
nsPlainTextSerializer::AppendText()
nsDocumentEncoder::SerializeNodeStart()
nsDocumentEncoder::SerializeRangeToString()
nsDocumentEncoder::EncodeToString()
nsCopySupport::HTMLCopy()
nsAutoCopyService::NotifySelectionChanged()
nsTypedSelection::NotifySelectionListeners()
nsSelection::NotifySelectionListeners()
nsSelection::SetMouseDownState()
nsTextInputSelectionImpl::SetMouseDownState()
nsFrame::HandleRelease()
nsFrame::HandleEvent()
PresShell::HandleEventInternal()
PresShell::HandleEvent()
nsViewManager::HandleEvent()
nsView::HandleEvent()
nsViewManager::DispatchEvent()
HandleEvent()
nsWidget::DispatchEvent()
nsWidget::DispatchWindowEvent()
nsWidget::DispatchMouseEvent()
nsWidget::OnButtonReleaseSignal()
nsWindow::OnButtonReleaseSignal()
nsWindow::HandleGDKEvent()
dispatch_superwin_event()
handle_gdk_event()
libgdk-1.2.so.0 + 0x17d7f (0x4035ed7f)
libglib-1.2.so.0 + 0x11773 (0x40391773)
libglib-1.2.so.0 + 0x11d39 (0x40391d39)
libglib-1.2.so.0 + 0x11eec (0x40391eec)
libgtk-1.2.so.0 + 0x94333 (0x402ad333)
nsAppShell::Run()
nsAppShellService::Run()
main1()
main()
libc.so.6 + 0x1c507 (0x404d8507) 

Comment 32

[Tanu Mutreja]

Michael, I can see the patch merged in both the following branches. 

http://lxr.mozilla.org/mozilla1.0/source/content/base/src/nsPlainTextSerializer.
cpp
http://lxr.mozilla.org/seamonkey/source/content/base/src/nsPlainTextSerializer.c
pp

With these changes I just could not reproduce the crash when I tried it last 
time. Would you please confirm that even after this patch it's reproducible 
with the same steps.

Comment 33

[Michael Smith]

Ok, to be completely clear about this:

Yesterday, I downloaded Mozilla 1.0 RC2 using the link on the mozilla.org home page:
http://ftp.mozilla.org/pub/mozilla/releases/mozilla1.0rc2/mozilla-i686-pc-linux-gnu-1.0rc2-sea.tar.gz

I untarred it and installed it into /usr/local

I ran /usr/local/mozilla/mozilla 

I checked "about:" and it stated:
Mozilla 1.0 Release Candidate 2
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.0rc2) Gecko/20020510

I went to http://www.m-w.com
I typed in "asd" into the dictionary search form field.
I submitted the form
Before the resulting page could be loaded, I double clicked on the "asd"

It crashed. 

So, yes, as I have stated in comments 18, 20, 24, 26, and 29, this is still
reproducible for me using the exact same steps described in the bug report.

Comment 34

[Andrew Schultz]

I'm also still crashing here with Mozilla binaries (20020515 trunk), but not
with optimized trunk CVS.

Comment 35

[Tanu Mutreja]

I checked that the code for Mozilla 1.0 RC2 has the patch merged to it. 
As I'm not able to do the proper testing at present, I would request someone 
else to look into it but based on what I remember about the testing for this 
patch it was no more reproducible after checking the patch in. May be something 
is still missing!

Comment 36

[Michael Smith]

Still reproducible for me in Mozilla 1.0 Release Candidate 3
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.0rc3) Gecko/20020523
Talkback ID: TB6750641E

Note:  I tried the latest nightly rather than the release candidate, and that
does not crash: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.0.0+) Gecko/20020527

Seems to me that the patch did *not* make it onto the branch.

Comment 37

[Andrew Schultz]

I can confirm that current trunk binary build (20020528) does not crash.  build
20020521 crashes, so the checkin here did not fix the crash.

Comment 38

[Andrew Schultz]

on the trunk, this un-regressed sometime between 2002052407 and 2002052421.
branch build 2002052809 still crashes

the only thing that looks remotely relevant is the checkin for bug 142562 which
touched quite a few of the related files, although the bug itself seems unrelated.

Comment 39

[Michael Smith]

This was previously targetted at mozilla 1.0 but was retargetted to 1.0.1 
because a fix was supposedly applied for 1.0; however, comment 38 indicates 
that I'm not the only one that is still seeing this crash on the release 
branch.  

I'm retargetting back to Mozilla 1.0 to hopefully get this bug back on 
someone's radar for 1.0.  Retarget for 1.0.1 if you must, I just want to make 
sure people realize this bug was not actually fixed in the release branch.

Comment 40

[Boris Zbarsky [:bzbarsky]]

1.0 is basically out the door (checkins are no longer being taken) and there is 
no fix here yet... please don't set unrealistic target milestones for 
developers (only their managers get to do that.  ;))

Comment 41

[Andrew Schultz]

Attachment: stacktrace from egcs/branch CVS

the tinderbox binaries crash because they're using egcs...

note that instead of the range going from -huge to 0, it goes from 0 to huge. 
that's why the bandaid-patch is not helping.

Comment 42

[Jay Patel [:jay]]

Michael:  Have you been able to reproduce this crash with the original testcase?
 I have not been able to reproduce this with any recent MozillaTrunk or Gecko1.0
Branch builds and there are 0 incidents in Trunk and Branch Talkback data.

So it looks like the bandaid patch works for that one case...but I guess we're
still looking for the real fix?  Should we mark this bug fixed (since the
original testcase no longer crashes) and open a new bug for the work needed for
the real fix?

Comment 43

[Jay Patel [:jay]]

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

Comment 44

[Jay Patel [:jay]]

Adding topcrash+ and testcase keywords from duped bug 146110.  This was a
topcrash at some point and *was* easily reproducible.  Also putting Trunk M1BR
[@ CopyChars1To2] [@ nsStrPrivate::EnsureCapacity] in summary since people have
crashed with both of those stack signatures.

Comment 45

[Andrew Schultz]

I'm still crashing here with branch 1.0 CVS and egcs.

Comment 46

[Jan Carpenter]

The last occurance in talkback of the stack signature
nsStrPrivate::EnsureCapacity was on build NetscapeGecko1.0Win322002112016 
And the last occurace of CopyChars1To2 was even longer ago on build
NetscapeNetscape6.21LinuxIntel2001112617 

Neither stack trace is showing up in the reports anymore.  

Comment 47

[Jan Carpenter]

Marking worksforme

Comment 48

[Andrew Schultz]

this crash is not showing up in talkback because this bug only exists on the 1.0
branch and (apparently) nobody uses branch builds.  I tested Mozilla 1.0.2 and
it still crashes.

also, the root problem in this bug still exists on the trunk (see comment 13). 
the patch here just covered up the crash.