267627 - select event isn't fired when selection of listbox is changed

Status: VERIFIED FIXED

(Core :: XUL, defect)

Description

[alexander :surkov (:asurkov)]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7) Gecko/20040616
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7) Gecko/20040616

I have listbox with seltype='multiple'. I select some items. Select event isn't
fired when I click on some selected item. In other words when I reduce selection
of listbox by clicking on selected item then event isn't fired.

I guess there is problem in click event handler of listitem binding.

var didSuppressSelect = false;
var i = 0;
while (i < selectedItems.length) {
  if (selectedItems[i] != this) {
    if (!didSuppressSelect) {
      parentNode._suppressOnSelect = true;
      didSuppressSelect = true;
    }
    parentNode.removeItemFromSelection(selectedItems[i]);
  }
  else
    i++;
}

When _suppressOnSelect=true then select event isn't fired.
Need to switch _suppressOnSelect and fire event after all calls of methods
removeItemFromSelection().

Reproducible: Always
Steps to Reproduce:

Comment 1

[Gérard Talbot]

> Select event isn't fired when I click on some selected item. 

Select event applies only for input type text and password and when
selecting/highlighting text.
http://www.w3.org/TR/html4/interact/scripts.html#adef-onselect

> In other words when I reduce selection
> of listbox by clicking on selected item then event isn't fired.
> 
> I guess there is problem in click event handler of listitem binding.

Click and change events are also different events.
http://www.w3.org/TR/html4/interact/scripts.html#adef-onchange

Can you attach a *reduced* testcase showing the problem?
What is a Simplified Test Case, and How Do I Make One?
http://www.mozilla.org/newlayout/bugathon.html#testcase

Comment 2

[alexander :surkov (:asurkov)]

>Select event applies only for input type text and password and when
>selecting/highlighting text.

I know what DOM Events Level2 specification says about select event. But I seem
XUL has extended event model. See xulplanet
http://www.xulplanet.com/references/elemref/ref_EventHandlers.html#attr_onselect.
See realization of listitem binding (it fires select event!): 
chrome://global/content/bindings/listbox.xml#listitem.

>Click and change events are also different events
I didn't told about change event I told about select event. Also I told that
listitem binding fires select event on click event.

Comment 3

[alexander :surkov (:asurkov)]

Attachment: testcase

Comment 4

[alexander :surkov (:asurkov)]

We can follow DOM Level2 Events specification and can remove firing of select
event from listbox element. In this case developers cannot know about selection
changes immediately (they will have chnage event only).

But we can serve extention of w3c specification for xul elements and we can add
one line in click hangler of listitem element.

if (didSuppressSelect)
            parentNode._suppressOnSelect = false;

parentNode._fireOnSelect();//we can add this line only to fire select event

Comment 5

[alexander :surkov (:asurkov)]

Need to write:

if (didSuppressSelect){
            parentNode._suppressOnSelect = false;
            parentNode._fireOnSelect();
}

Comment 6

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Attachment: patch

See comment #5.  This makes the testcase work as the reporter expects.

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 166293 [details] [diff] [review]
patch

Brian might have more of a clue about this than either dbaron or I.

Comment 8

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

checked in by timeless

Comment 9

[Stephen Donner [:stephend] Not actively reading bugmail]

https://bugzilla.mozilla.org/attachment.cgi?id=164689&action=view is working
fine for me now with build 2004-12-07-06 on Windows XP.

Tried all types of selection, including contiguous and non-contiguous.

Verified FIXED

Comment 10

[dmitry rusanov]

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

Comment 11

[Nickolay_Ponomarev]

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