172298 - onselect="this.select();" in textarea causes crash (stack overflow) [@nsXULElement::HandleDOMEvent]

Status: RESOLVED FIXED

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

Description

[Neil Fraser]

Add the following to a textarea in any web page:
  onselect="this.select();"
Select some text in the textarea.
Watch Mozilla crash.

Comment 1

[Neil Fraser]

Attachment: Minimal demonstration of crash

Adding attachment wich causes the crash.

Mozilla version: 2002091014

There should also be a traceback with ID of: TB11964008M

Comment 2

[David Bradley]

Attachment: Stack for TB ID 11964008

This was a stack overflow in that terminated in nsXULElement::HandleDOMEvent

Comment 3

[Nick Lott]

Can confirm in build 2002100108 on W2K (TB ID 11965432Q)

Comment 4

[Neil Fraser]

BTW, this isn't a "who'd-ever-do-such-a-stoopid-thing" bug.  Under IE6, this
little bit of code selects the entire contents of a textarea when you select
anything.  So it makes selection be all or nothing.  Quite handy if used in the
right places.  Quite obnoxious if used in others.

Comment 5

[Frank Wein [:mcsmurf]]

Confirming with Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.2b)
Gecko/20021002

From Dr.Watson:

eax=06718e24 ebx=01acd112 ecx=00000002 edx=06718e24 esi=0012f7e0 edi=0012f648
eip=60823c7f esp=0012f610 ebp=0012f624 iopl=0         nv up ei pl nz na po nc
cs=001b  ss=0023  ds=0023  es=0023  fs=0038  gs=0000             efl=00200206


Funktion: <nosymbols>
        60823c65 b857000780       mov     eax,0x80070057
        60823c6a eb18             jmp     6082c784
        60823c6c 8b442404         mov     eax,[esp+0x4]         
ss:00bbcbe3=????????
        60823c70 8b5018           mov     edx,[eax+0x18]        
ds:071a63f6=????????
        60823c73 8911             mov     [ecx],edx             
ds:00000002=????????
        60823c75 8b4018           mov     eax,[eax+0x18]        
ds:071a63f6=????????
        60823c78 85c0             test    eax,eax
        60823c7a 7406             jz      6082c782
        60823c7c 8b08             mov     ecx,[eax]             
ds:06718e24=00000002
        60823c7e 50               push    eax
ERROR ->60823c7f ff5104           call    dword ptr [ecx+0x4]   
ds:00a8d5d4=????????
        60823c82 33c0             xor     eax,eax
        60823c84 c20800           ret     0x8
        60823c87 8b442404         mov     eax,[esp+0x4]         
ss:00bbcbe3=????????
        60823c8b 8b4c2408         mov     ecx,[esp+0x8]         
ss:00bbcbe3=????????
        60823c8f 894818           mov     [eax+0x18],ecx        
ds:071a63f6=????????
        60823c92 33c0             xor     eax,eax
        60823c94 c20800           ret     0x8
        60823c97 55               push    ebp
        60823c98 8bec             mov     ebp,esp
        60823c9a 83ec14           sub     esp,0x14
        60823c9d 53               push    ebx

*----> Stack Back Trace <----*

FramePtr ReturnAd Param#1  Param#2  Param#3  Param#4  Function Name
0012F610 60E7BD74 00F2E2B0 0012F648 0012F648 0012F7C4
embedcomponents!<nosymbols>  (FPO: [2,0,0])
0012F624 60111932 00F2E2B0 0000000C 00000001 0012F648 xpcom!XPTC_InvokeByIndex 
0012F7C4 60114E23 01000001 9210B958 00000000 063C3AC8 xpc3250!<nosymbols> 
0012F854 600BA80E 063C3AC8 05B10A68 00000000 05B10AEC xpc3250!<nosymbols> 
0012F904 600BAAA4 00000001 00000000 00000002 0012FB2C js3250!js_Invoke 
0012F984 600C7935 0683504C 0310B958 0310BA50 00000000 js3250!js_Invoke 
0012F9C8 600BEC6D 063C3AC8 0310B958 01BE6748 0012FB2C js3250!js_FindProperty 
0012FB34 600BA84B 063C3AC8 0012FBBC 00000002 0012FBF8 js3250!js_Invoke 
0012FBD8 600BAAA4 00000001 00000002 00000002 80000000 js3250!js_Invoke 
0012FC58 600A47BC 063C3AF0 02C02570 02E41E18 00000000 js3250!js_Invoke 
0012FC80 604F3CCF 063C3AC8 02C02570 02E41E18 00000002 js3250!JS_CallFunctionValue 
0012FCC8 604FB6E2 00EEFC88 02C02570 02E41E18 00000002 jsdom!<nosymbols> 
0012FE20 604FBB6C 00000000 0671B480 60E6F827 0671B480 jsdom!<nosymbols> 
0012FE2C 60E6F827 0671B480 046F74D8 00F7531C 60E6FBB9 jsdom!<nosymbols>  (FPO:
[2,0,1])
0012FE4C 60538858 00F75310 80000000 00000000 00000000 xpcom!NS_NewThreadPool 
(FPO: [0,0,3])
0012FE9C 6095720E 00F75310 00401F9F 00F45668 780419E0 gkwidget!<nosymbols> 
0012FEA4 00401F9F 00F45668 780419E0 00000000 00000000 appshell!<nosymbols> 
(FPO: [1,0,0])
0012FEEC 00401B25 00000001 00233A30 00233A88 780419E0 mozilla!<nosymbols> 
0012FF14 00403550 00000001 00233A30 00133894 0040746C mozilla!<nosymbols> 
0012FF24 0040746C 00400000 00000000 00133894 00000001 mozilla!<nosymbols>  (FPO:
[4,0,1])
0012FFC0 77E8CA90 002E0061 00780065 7FFDF000 C0000005 mozilla!<nosymbols> 
0012FFF0 00000000 00407338 00000000 000000C8 00000100 kernel32!CreateProcessW 

*----> Raw Stack Dump <----*
0012f610  24 8e 71 06 74 bd e7 60 - b0 e2 f2 00 48 f6 12 00  $.q.t..`....H...
0012f620  48 f6 12 00 c4 f7 12 00 - 32 19 11 60 b0 e2 f2 00  H.......2..`....
0012f630  0c 00 00 00 01 00 00 00 - 48 f6 12 00 00 00 00 00  ........H.......
0012f640  5c 50 83 06 9c f8 12 00 - 24 8e 71 06 06 00 00 00  \P......$.q.....
0012f650  48 f6 12 00 92 05 23 00 - 70 d9 23 00 06 00 00 00  H.....#.p.#.....
0012f660  08 00 00 00 10 03 23 00 - 10 03 23 00 64 66 d1 02  ......#...#.df..
0012f670  04 00 00 00 00 00 23 00 - 98 02 23 00 98 02 23 00  ......#...#...#.
0012f680  04 00 00 00 10 00 00 00 - 98 02 23 00 03 00 00 00  ..........#.....
0012f690  58 54 ea 00 a8 f8 12 00 - 2b 8c e7 60 c0 02 23 00  XT......+..`..#.
0012f6a0  c0 02 23 00 05 00 00 00 - 02 00 00 00 c0 02 23 00  ..#...........#.
0012f6b0  18 00 00 00 05 00 00 00 - 02 00 00 00 c0 02 23 00  ..............#.
0012f6c0  c0 02 23 00 95 2b 88 77 - 20 96 11 60 d8 f6 12 00  ..#..+.w ..`....
0012f6d0  00 00 00 00 3f 00 00 80 - 00 00 12 00 d0 f7 12 00  ....?...........
0012f6e0  04 f7 12 00 f6 2c 10 60 - 14 eb b5 01 fc f6 12 00  .....,.`........
0012f6f0  78 f7 12 00 c8 f7 12 00 - 58 b9 10 03 ec 0a b1 05  x.......X.......
0012f700  89 0d 01 00 44 f7 12 00 - 35 29 10 60 14 eb b5 01  ....D...5).`....
0012f710  c8 f7 12 00 c4 f7 12 00 - 00 00 00 00 d0 f7 12 00  ................
0012f720  90 f7 12 00 78 f7 12 00 - b8 f7 12 00 5d 28 10 60  ....x.......](.`
0012f730  14 eb b5 01 c8 3a 3c 06 - 00 00 00 00 14 eb b5 01  .....:<.........
0012f740  c8 3a 3c 06 ec f7 12 00 - 52 35 11 60 3e 13 10 60  .:<.....R5.`>..`

Comment 6

[Phil Schwartau]

Browser, not engine ---> DOM Level 0

See Comment #4 for the IE behavior on this - 

Comment 7

[Madhur Bhatia]

getting a browser hang ... no crash on linux 7.2 -- build 2002-10-22-08trunk.

Comment 8

[Olivier Cahagne]

Attachment: Stack trace build 20021123 (CVS) on Linux

Comment 9

[Miha Psenica]

I produce a crash every time when I scroll with mouse button inside a textarea
(it uses a lot of css formating and a javascript function focus()).

This has been happening since 1.1, maybe also before that, and is still visible
in 1.3b under Win98.

Look at bug 176257 and bug 183181 also.

Comment 10

[Miha Psenica]

Confirming crash from the field in attachment #1 [details] [diff] [review].
I'm using Mozilla 2003021304 but has been happening at least from 1.1. My OS is
Win98.

I find this bug very irritating so I hope someone will at least look into it.

Comment 11

[Harshal Pradhan]

Attachment: Make mozilla not recurse to death (and behave like IE)

This simply stops the recursion.

Comment 12

[John Keiser (jkeiser)]

Comment on attachment 116696 [details] [diff] [review]
Make mozilla not recurse to death (and behave like IE)

All right, this seems fine.  The proper alternative, to stop this stuff more at
the root in EventStateManager, are a long way off.

Comment 13

[kinmoz]

Comment on attachment 116696 [details] [diff] [review]
Make mozilla not recurse to death (and behave like IE)

This looks fine to me. Do we need to handle nsHTMLInputElements too?

Comment 14

[John Keiser (jkeiser)]

Comment on attachment 116696 [details] [diff] [review]
Make mozilla not recurse to death (and behave like IE)

Nope, nsHTMLInputElement already handles it--though in a slightly different
way; it will prevent the second select event from ever being dispatched, which
means you couldn't return false from onselect and prevent the select() from
going through.

Thinking about it, I think the right way to do this (that actually *does* give
parity with IE) is to mimick <input>'s way of doing it.  Basically don't allow
a second select event to propagate at all.  Then you don't have a chance of
re-running other JavaScript on accident.

Comment 15

[John Keiser (jkeiser)]

Attachment: Testcase Showing Events

Run this testcase in IE to see the number of select events.  Harshal, you want
to take a shot at this?

Comment 16

[Harshal Pradhan]

I'm not going to have the time for the next couple of weeks. I'll take a look 
if this is not fixed by that time.

Comment 17

[Johnny Stenback (:jst)]

Mass-reassigning bugs to [e-mail]

Comment 18

[Harshal Pradhan]

Attachment: Prevent dispatch of second select event

Comment 19

[Harshal Pradhan]

Comment on attachment 120972 [details] [diff] [review]
Prevent dispatch of second select event

John, is this close to what you had in mind?

Comment 20

[John Keiser (jkeiser)]

Comment on attachment 120972 [details] [diff] [review]
Prevent dispatch of second select event

Precisely, but since this is a PRPackedBool, let's set explicitly to PR_TRUE
and PR_FALSE instead of bit twiddling--bit twiddling on bools and assuming they
are ints is highly discouraged.  if (isSelectEvent) { mHandlingSelect =
PR_TRUE; } etc.  r=me with that.

Comment 21

[Harshal Pradhan]

-> To me.

Comment 22

[Harshal Pradhan]

Attachment: Same thing, but without any bit-twiddling

This is what I had first, but it seemed a little bit ugly so I did the
bittwiddling thing. I wasnt aware there were issues with doing that with
PRPackedBool.

Comment 23

[John Keiser (jkeiser)]

Comment on attachment 121115 [details] [diff] [review]
Same thing, but without any bit-twiddling

Thanks!  I really appreciate the patch.

Comment 24

[John Keiser (jkeiser)]

The problem with bit-twiddling PRBool or PRPackedBool is not a practical one but
a theoretical one, we basically want to ensure 100% that PR_TRUE (1) and
PR_FALSE (0) are the only values they get set to because some callers of
functions check == PR_TRUE.  Bit-twiddling introduces the possibility of doing
that (though admittedly not in this case).

Comment 25

[kinmoz]

Comment on attachment 121115 [details] [diff] [review]
Same thing, but without any bit-twiddling

sr=[e-mail]

Comment 26

[Harshal Pradhan]

Checked in.