http://code.google.com/p/chromium/issues/detail?id=145489 If a web page opens alert() or something in a 'change' event handler of input[type=number] and a user press the spin button in it, a modal dialog opens but mouse events are still captured by the spin button. We should release mouse event capturing in such case.
Created attachment 166602 [details] Patch
Comment on attachment 166602 [details] Patch Is this frame local? We could add a listener/observer on Chrome object for detecting willOpenMocalDialog(), and let SpinButtonElement to implement that listener and register itself to Chrome object. Adding a virtual method to Node only for this purpose looks a bit too abusive for me.
Comment on attachment 166602 [details] Patch I don’t like the assumptions here. Not all clients will use modal dialogs for things like a JavaScript alert. And many clients will use modal dialogs in other situations, like for a file chooser in an input element. It would be better if we did not have to hardcode the assumption that a modal dialog will come up, but could instead respond to the fact that one did come up.
(In reply to comment #3) > (From update of attachment 166602 [details]) > I don’t like the assumptions here. Not all clients will use modal dialogs for things like a JavaScript alert. And many clients will use modal dialogs in other situations, like for a file chooser in an input element. It would be better if we did not have to hardcode the assumption that a modal dialog will come up, but could instead respond to the fact that one did come up. Yeah, that is my point. If Chrome object can notify the fact, then native implementation which uses mousemove can register itself to handle modal dialogs regardless who does it.
Comment on attachment 166602 [details] Patch Yeah, we need a generalized fix. http://code.google.com/p/chromium/issues/detail?id=154329 ':hover isn't cleared for alert() or confirm()' is a similar problem.
Created attachment 172727 [details] WIP
(In reply to comment #6) > Created an attachment (id=172727) [details] > WIP I still need to add a function to Node in this new patch because EventHandler assumes m_capturingMouseEventsNode is Node. > could instead respond to the fact that one did come up. IMO, it's ok to cancel capturing on any events which *might* open dialogs/popups.
Created attachment 174066 [details] Patch 2
CC-ing haraken. This addition might increase sizeof(Node) but I'm sure if it really does. Haraken, do you remember the bottom line of past discussion?
Comment on attachment 174066 [details] Patch 2 View in context: https://bugs.webkit.org/attachment.cgi?id=174066&action=review > Source/WebCore/dom/Node.h:125 > +class Node : public EventTarget, public ScriptWrappable, public TreeShared<Node, ContainerNode>, public PopupOpeningObserver { Yeah, this will increase sizeof(Node) by 8 byte, which wouldn't be acceptable. A virtual function table pointer is needed for each multiply-inherited class that has virtual functions. Currently only EventTarget has virtual functions and thus each Node has only one virtual function table pointer. If you add PopupOpeningObserver, which has virtual functions, it will add one more virtual function table pointer to each Node.
Comment on attachment 174066 [details] Patch 2 View in context: https://bugs.webkit.org/attachment.cgi?id=174066&action=review >> Source/WebCore/dom/Node.h:125 >> +class Node : public EventTarget, public ScriptWrappable, public TreeShared<Node, ContainerNode>, public PopupOpeningObserver { > > Yeah, this will increase sizeof(Node) by 8 byte, which wouldn't be acceptable. > > A virtual function table pointer is needed for each multiply-inherited class that has virtual functions. Currently only EventTarget has virtual functions and thus each Node has only one virtual function table pointer. If you add PopupOpeningObserver, which has virtual functions, it will add one more virtual function table pointer to each Node. I see. I'll update the patch so that I don't change Node parents.
Created attachment 174088 [details] Patch 3 Node doesn't inherit PopupOpeningObserver
Comment on attachment 174088 [details] Patch 3 This version is much better than before!
Comment on attachment 174088 [details] Patch 3 Clearing flags on attachment: 174088 Committed r134886: <http://trac.webkit.org/changeset/134886>
All reviewed patches have been landed. Closing bug.