mozilla crashes after selecting text input field and pressing a key

VERIFIED FIXED in mozilla1.0

Status

()

Core
Selection
--
critical
VERIFIED FIXED
17 years ago
15 years ago

People

(Reporter: Christoph Studer, Assigned: mjudge)

Tracking

({crash})

Trunk
mozilla1.0
x86
Windows 2000
crash
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: EDITORBASE+, FIXINHAND, URL)

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

17 years ago
Go to the URL. On the top of the page you see an input field with the value "test". 
1 . Now point with the cursor at the upper border of the text field between "te"
and "st".
2. triple click to select the underlying paragraph. 
3. if mozilla only selects the
text field's content, move the cursor pixel by pixel vertically&horizontally an
goto 2
until the text before and the space after the text field is selected.
3. type any key

Result: mozilla crashes
Expected: nothing. maybe a "bing"-sound to indicate that the nothing happens
though the user pressed a key.

note that this does not happen by selecting the text as you would normally by
clicking and dragging over it.

I actually encountered this on in a slightly different kind:
1. go to https://wwws.nic.ch/reg/domain/searchdomain.cfm
2. enter "test" into the text field.
3. select the text in the same way as described above.
4. repeat 3 until the content plus the surrounding text is selected.
5. type key

Result: mouzilla replaces field's content and deletes surrounding text
Expected: nothing. maybe a "bing"-sound to indicate that the nothing happens
though the user pressed a key.

Sorry for choosing "Browser-General". Please tell me if I sould split this bug
into two.

Updated

17 years ago
Severity: normal → critical
Keywords: crash
Reporter:
Please always add the build ID in a bug report.
for the crash:
Plase use a talkback enabled build. After you crashed, talkback should pop up.
After talkback submitted the crash to the server run
mozilla/components/talkback.exe and add the Talkback ID# from your crash in this
bug.

Thanks
(Reporter)

Comment 2

17 years ago
sorry for the lack of this information.

Build ID:    2002020103
Incident ID: TB2425922Y
Keywords: stackwanted
0x00000000
nsTypedSelection::EndBatchChanges
[d:\builds\seamonkey\mozilla\content\base\src\nsSelection.cpp, line 7520]
nsEditor::Do [d:\builds\seamonkey\mozilla\editor\libeditor\base\nsEditor.cpp,
line 502] 
-> selection
Assignee: asa → mjudge
Status: UNCONFIRMED → NEW
Component: Browser-General → Selection
Ever confirmed: true
Keywords: stackwanted
QA Contact: doronr → tpreston
(Assignee)

Comment 5

16 years ago
i can reproduce this. i am working on it. I think this is EDITORBASE.
Status: NEW → ASSIGNED
Whiteboard: EDITORBASE
(Assignee)

Comment 6

16 years ago
ok i fixed it.  the problem was that the tripple click actually.  The code was 
written such that it didnt go through nsISelection. Instead it went directly to 
the nsITypedSelection and called collapse and Extend.  collapse and extend 
never before checked the "limiter" of their container nsISelection.  AND since 
the text fields are technically in the same document it was perfectly legal in 
a dom sense to set the selection to before or extend to after the text field.  
Thus when a key was typed it tried to delete and reflow content outside the 
text field and booom!

I fixed this by calling up to the containing frameselection from the 
nsTypedSelection (if a frame selection exists)  and I ask for the Limiter.  The 
limiter is a div that surrounds the anonymous content of the text field/area.  
This method was allready in place.   IF such a limiter exists we check it 
against the passed in content and offsets.  If the passed in content nor the 
parent of that content(aka a text node check the parent) is the limiter then we 
have a problem and some frame is trying to set its nsTypedSelection to a bogus 
location and we simply fail out as if no extend nor a collapse was called.  

Patch to be attached.
(Assignee)

Comment 7

16 years ago
Created attachment 71999 [details] [diff] [review]
fix to check the limiter of the parent nsSeleciton

fix for crasher
(Assignee)

Comment 8

16 years ago
talked to syd he has said this is +'d. looking for r= and sr=
Whiteboard: EDITORBASE → EDITORBASE+, FIXINHAND
Target Milestone: --- → mozilla1.0

Comment 9

16 years ago
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 adt@netscape.com.  You can search for
"Moving bugs not scheduled for a project" to quickly delete this bugmail.
Target Milestone: mozilla1.0 → mozilla1.2

Comment 10

16 years ago
Comment on attachment 71999 [details] [diff] [review]
fix to check the limiter of the parent nsSeleciton

r/sr=kin@netscape.com (whichever you need) with the following changes:


-- Since the 2 pieces of code you are adding are identical,
   perhaps a utility method would be good. I'd also add
   comments to that method about why you can make the
   assumptions you make about tLimiter, and the fact that
   you only have to match the container's parent and grandparent
   against it.



+  nsCOMPtr<nsIContent> tLimiter;
+  if (mFrameSelection)
+  {


-- Since tLimiter isn't used outside of the |if| block, I'd
   move it's declaration into it.


+    result = mFrameSelection->GetLimiter(getter_AddRefs(tLimiter));
+    if (NS_FAILED(result))
+      return result;


-- Does GetLimiter() guarantee a non-null tLimiter if result is not
   a FAILED code? if not perhaps we should check tLimiter for null.
Attachment #71999 - Flags: superreview+
(Assignee)

Comment 11

16 years ago
tlimiter WAS being checked for null.  It is perfectly valid not to have a 
limiter in the case of topmost webbrowser or composer. 

I have factored out the method and had the original check for the limiter also 
call the factored out function.  I put in a big comment on what the heck this 
is all used for as well. new patch
(Assignee)

Comment 12

16 years ago
Created attachment 73905 [details] [diff] [review]
patch for nsSelection.cpp in content/base/src.

factored out function, better comments
Attachment #71999 - Attachment is obsolete: true

Comment 13

16 years ago
Comment on attachment 73905 [details] [diff] [review]
patch for nsSelection.cpp in content/base/src.

sr=kin@netscape.com

Perhaps we should also mention (with an XXX comment?) that should the need ever
arise for a limiter on a non-text-widget, that IsValidSelectionPoint() would
have to be modified to traverse the container's parent hierarchy with a loop
looking for tLimiter.
Or if you're feeling frisky you could just add that loop. :-)
Attachment #73905 - Flags: superreview+

Comment 14

16 years ago
Comment on attachment 73905 [details] [diff] [review]
patch for nsSelection.cpp in content/base/src.

r=blythe
Attachment #73905 - Flags: review+
(Assignee)

Comment 15

16 years ago
ok I put in a comment about the while loop. I dont want to put it in because i 
think its unecessary. r=blythe sr=kin i need approval too.

I have had Sujay Desai pound on the build with this change on it for about 45 
min.  He reported no obvious problems if this helps get the a=
(Assignee)

Comment 16

16 years ago
i also ran the precheckin tests, sent mail, open files.

Comment 17

16 years ago
Comment on attachment 73905 [details] [diff] [review]
patch for nsSelection.cpp in content/base/src.

a=asa (on behalf of drivers) for checkin to the 1.0 trunk
Attachment #73905 - Flags: approval+

Updated

16 years ago
Target Milestone: mozilla1.2alpha → mozilla1.0
(Assignee)

Comment 18

16 years ago
checked in didnt comment on it
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
no longer crashes --tested with 2003.02.27.09 comm trunk, win2k.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.