46085 - The onselect event handler does not work

Status: VERIFIED FIXED

(Core :: DOM: UI Events & Focus Handling, defect, P2)

Description

[Marcell]

The onselect event handler on the text and textarea objects do not work in 
Mozilla.

Steps to Reproduce: 
1. Open attachment.
2. Select the text in the text field and/or textarea.

Actual Results: The onselect event handler was not triggered; nothing happened.

Expected Results: The onselect event handler should have been triggered and 
should have popped up an alert box.

Tested on MacOS 8.6, MacOS 9, Win98, WinNT, Win2000, Linux Redhat 6.0 on build 
2000-07-20-21-M17

Comment 1

[Marcell]

Attachment: Testcase

Comment 2

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Where's the work to do this?  Events, form controls, or editor?  (or elsewhere?)

Comment 3

[ckritzer (gone)]

Mass update:  changing qacontact to [e-mail]

Comment 4

[ckritzer (gone)]

PDT:  Nominating for nsbeta3+ on Standards Issue.

Comment 5

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

I am the virtual joki.

Comment 6

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

Per discusion with Nisheeth, marking nsbeta3+. Will email ekrock to verify.

Comment 7

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

Attachment: First cut fix: fire the select event

Comment 8

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

It seems like we have all/almost all the code to handle the select event. We 
just aren't creating the select event. The first cut fix is suboptimal in some 
ways:

1) It creates a select event, but I am not sure if it is the correct one (seems 
to work).
2) The place where this event is created may not be the best of places.
3) The event is created when selection changes, which is wrong. It should be 
created when selection is "finished", i.e. on mouseup, keyup etc. The function 
where I create the event gets a reason parameter but it cannot be trusted, for 
example the reason is never MOUSEUP_REASON.
4) HTML4 says onselect attribute may be used with INPUT and TEXTAREA elements, 
but with my patch applied it seems you can use that attribute on any element. 
For example, I placed onselect attributes on INPUT, its parent FORM, its parent 
BODY, and they all got fired (the event bubbles as it should do, but I do not 
think you should be able to capture it with onselect attribute on any other 
element except INPUT and TEXTAREA).

Comment 9

[Eric Pollmann]

1) NS_FORM_SELECTED is the right event to send.
2-4) Would be nice to fix, but I don't know how.  What does IE do?

Bug 13652 is about the same event but is not really related to this fix.  These
two fixes should both go in!  :)

Comment 10

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

I spoke with vidur about 4), and he pointed out that we are doing this all over 
the place and this is sort of a requirement from the DOM perspective where you 
can attach event handlers to anything. Strictly speaking it shouldn't work by 
using the special attribute, but...

So this still leaves 2) and 3) open. As to 3), IE fires the event once you are 
done with selecting (i.e. you release the mouse button). That is the right thing 
to do in my opinion. I will need to chat with mjudge how to do these.

Comment 11

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

Attachment: Fix to layout

Comment 12

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

Attachment: Fix to dom

Comment 13

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

Mike, can you review?

I added KEYPRESS_REASON to be able to reliably follow how the selection changes 
with keys. I did not need to change any event handling code, all the machinery 
seems to be there. I just need to fire the event at the right time (from the 
right place?).

There are some differences with IE. It would be quite difficult to follow IE 
compeletely (and besides it has strange "features" e.g. when you extend the 
selection over a space it starts selecting words), and the current behaviour 
seems more natural to us (spoke with jst & vidur).

I haven't seen anything breaking because of this (I have had something like this 
in my tree for 5 days now). I have not yet run full prechekin tests, but they do 
not have much in the way of testing selection. I guess you would know if there 
is something suspicuous (for example, do you think there is any impact on the 
Linux select/paste?)

Comment 14

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

r=mjudge, thanks...

Comment 15

[Stephen Walker]

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

Comment 16

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

Fix checked in.

Comment 17

[Diego]

It works. Linux 2000090208 nightly.
Tested also bug 50094 (dupe of this one)

Tested also on Solaris 7 2000083113

Comment 18

[Loco]

Verified working on Mac OS 9.0.4 build 2000-09-04-08.  Also verified in Win 98 
build 2000-09-06-08.  Awaiting Linux verification and win 2000 to close it out.

Comment 19

[Loco]

Linux is now verified with 2000-09-06-08.  Awaiting final Win 2k.

Comment 20

[Loco]

Tested and passed in Win2k.

On the other hand, as Hixie pointed out, this reaction should also happen if the 
text in the box is selected by using Edit --> Select All.  That doesn't work in 
either Mac OS 9.0.4, Linux, Win98, or Win2k in the tested builds.  Someone care 
to clarify this for me?  If the response is that the Edit Menu should have no 
bearing here, we can close it, but I have to say Hixie won me over on that 
argument:).

Thanks.

Comment 21

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

Hmm, never tested select all. I'll have to take a look.

Comment 22

[ckritzer (gone)]

This is still broken.

Reopening on:
- LinuxRH62 2000-09-07-08-M18 Commercial
- Win98     2000-09-07-08-M18 Mozilla
- MacOS86   2000-09-07-04-M18 Commercial

Comment 23

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

So, the problem with Select All is that I are looking for a reason why selection 
listeners are notified. Without a correct reason I would send too many events. 
Currently I accept reasons MOUSEUP_REASON and KEYPRESS_REASON. In the case of 
Select All I get NO_REASON. I have a fix for this in my tree, but I am not too 
happy with it. The fix is to create a new method in nsIDOMSelection called 
selectAllChildren() which sets a new reason SELECTALL_REASON, and fix the 
patterns that called "collapse + extend all" to use the explicit method instead.

I think there is also a problem with a script selecting text. I have been unable 
to make a test document that has script selecting working.

I have also been working on something that would disregard reasons althogether, 
and be only concerned about the selection itself. This would solve the problems 
althogether. But that presents a new problem I haven't been able to solve yet. A 
selection can contain multiple ranges, and I do not know what range should I be 
tracking. That might be a problem even if we got reason parameters working 
perfectly. For example, we could have SCRIPT_REASON, but a script might add 
several ranges to the selection and change some of them that are not in the 
INPUT or TEXTAREA element so these should not send events.

I have also noticed some weirdness with the event.target. With KEYPRESS_REASON 
it is correct. With MOUSEUP_REASON event.target is [object Text]. With 
SELECT_ALL reason event.target is [object HTMLDocument].

Comment 24

[Eric Pollmann]

> I think there is also a problem with a script selecting text. I have been
> unable to make a test document that has script selecting working.

Hi Heikki, I just put some code in last night so that if you select from
javascript like: formA.inputB.select() a DOM select event will be fired.  The
logic to do this was added to nsHTMLInputElement::Select and
nsHTMLTextAreaElement::Select.  If you do come up with an alternate way of
firing select events through script-initiated selection, the logic I added may
need to be removed.

Comment 25

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

I didn't have luck with disregarding reasons althogether and monitoring only the 
selected content. We simply get too many times into this function. For example, 
we get here on each keypress and mousedown, even when there is no selection. And 
the other problem is that selection might contain multiple ranges in which case 
there doesn't seem to be a reliable way of dinding which range to look at.

So I guess that leaves us with the second best alternative and have a new 
reason, the SELECTALL_REASON.

Still I have been unable to test this with scripts. (I haven't tested the DOM 
select() method, I am trying to test the nsIDOMSelection collapse/extend 
methods). For some reason this does not work:

<script>
function mysel() {
  document.getElementById("id2").focus();	  
  window._content.getSelection().collapse(document.getElementById("dumle"),0);	  
  window._content.getSelection().extend(document.getElementById("dumle"),1);}
</script>

<input type="button" value="Script Select All Input Text Field" 
onclick="mysel()">  

<div id="dumle"> 
<INPUT id="id2" type="text" value="Select this"> 
</div>

If I can get a hold of mjudge I will probably check my fix in today.

I also found that if you have the caret at the beginning of the INPUT text field 
and press shift+end, or if you are in the end of the field and press shift+home, 
you will get TWO select events. I believe this is bug 37262.

Comment 26

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

Checked in the fix for Select All. I also fixed event.target. joki pointed out 
that nsIPresShell has a method called HandleEventWithTarget which will take care 
of pre- and posthandle event stuff, so now I am using that instead of directly 
calling the HandleDOMEvent of the content node.

Comment 27

[ckritzer (gone)]

Marking VERIFIED FIXED on:
- LinuxRH62 2000-09-13-08-M18 Commercial
- Win98     2000-09-13-08-M18 Mozilla
- MacOS86   2000-09-13-04-M18 Commercial