[Webkit-unassigned] [Bug 36177] [Qt] Multiselect Popup - Listbox click simulation

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=36177





--- Comment #16 from Luiz Agostini <luiz.agostini at openbossa.org>  2010-03-17 16:54:32 PST ---
(In reply to comment #15)
> (From update of attachment 50923 [details])
> The thing that I don't like here, is that you are making a method that
> simulates click regardless of it being a multiple selection or a normal one.

I think now that the word 'simulation' should be removed from the bug title. It
came from the fact that the very same code used in mose handling is been shared
here.

Anyway I think that the name of the method listBoxPopupClick make clear what
this method does and what it is used for.

> This seems a bit strange to me, as these are currently two different code
> paths. The question is if they should be that or if that could be refactored.

The need of two separated paths is not obvious to me and I think that the same
code could be used for all cases. But for now I would like to keep focus on
multiselect. Anyway this patch keeps the two different paths

Please consider that nothing really new is been implemented. Part of the mouse
handling code has been refactored and is now been reused here for <select
multiple> popups. All the other cases will handled as they were before to be
sure that no side effects will show up.

> Also multi and shift are very mouse specific, and for what you are trying to
> accomplish, you will never need multi nor shift,

As this implementation is not platform specific it could be used even with
mouses and keyboards. It is not mouse specific because it provides all the
needed features for touchable devices but it also does not exclude mouses.

Actually multi is exactly the needed parameter for <select multiple> popups. It
indicates if the selection of the list should turn to the pointed option only
(multi == false) or if just the select state of the pointed option should
toggle (multi == true).

> thus maybe it makes more sense to refactor updateSelectedState into a 
> 
> updateIndex(..., int optionIndex, bool fireOnChangeNow, ...) similar to 
> SelectElement::setSelectedIndex(SelectElementData, Element, optionIndex,
> deselect, fireOnChangeNow, userDrivenChange); 
> 
> and have updateSelectedState call that? 

I just think that it is not needed.

> I guess updateSelectedState does more than you need, like repaints the items,
> for instance in the case you "select" an item with a mouse without holding down the keyboard keys.

No repainting happens for menulists renders.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.