Bug 57858 – WebKit for Mac should always propagate a mouse release event to a subframe

• Component: WebCore Misc.
• Status: NEW

[Daniel Bates]

Currently on non-Mac ports mouse release events are propagated to a subframe regardless of whether the originating mouse press occurred in a subframe. In particular, a mouse release event would be propagated to a subframe F when dragging the divider of a non-resizable FrameSet over F and releasing the mouse. Instead we should only propagate the mouse release event to a subframe so long as both the release and corresponding mouse press event occurred in a subframe.

[Daniel Bates]

Created attachment 88269 [details]
Patch and layout tests

[Daniel Bates]

Comment on attachment 88269 [details]
Patch and layout tests

Clearing review flag so as to re-work the test case should-not-fire-mouseup-in-subframe-on-resize-when-frameset-prevents-resize.html so that it works in Chrome.

[Daniel Bates]

Created attachment 88282 [details]
Patch and layout tests

[Alexey Proskuryakov]

Comment on attachment 88282 [details]
Patch and layout tests

View in context: https://bugs.webkit.org/attachment.cgi?id=88282&action=review

I'm wondering if WebKit2 was affected.

> Source/WebCore/page/EventHandler.cpp:1344
> +        m_mouseDownWasInSubframe = true;

What will set it back to false?

[Daniel Bates]

After reading <http://dev.w3.org/2006/webapi/DOM-Level-3-Events/html/DOM3-Events.html#events-mouseevent-event-order>, we want the mouseup event to be propagated to the subframe.  That is, the behavior observed in the Apple Mac port disagrees with the DOM Level 3 spec as well as as is inconsistent with the behavior observed in the Apple Windows port.

In particular, the DOM Level 3 spec states "A user agent must dispatch [mouseup] when a pointing device button is released over an element."

Additional remarks:

The behavior observed in the Apple Mac port differs from the behavior observed in IE 5.5, 6, 7 and Firefox 3.6.16, which fire a mouseup event in a subframe when the mouse button is released over an element.

[Daniel Bates]

(In reply to comment #4)
> (From update of attachment 88282 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=88282&action=review
> 
> I'm wondering if WebKit2 was affected.
> 
> > Source/WebCore/page/EventHandler.cpp:1344
> > +        m_mouseDownWasInSubframe = true;
> 
> What will set it back to false?

I spoke with Alexey today (04/06/2011) on IRC regarding the observations I mentioned in comment 5. Disregard the patch posted on 04/05. See comment 5 for more details.

[Daniel Bates]

Created attachment 88545 [details]
Patch and layout test

I am not exactly clear on the purpose of m_mouseDownWasInSubframe. Alexey mentioned that this instance variable was added in r3003 <http://trac.webkit.org/changeset/3003>. From my understanding of this changeset and the current incarnation of this source code and its call sites, it seems that the use of m_mouseDownWasInSubframe is related to some idiosyncrasies with NSView.

There were no layout test regressions with this patch.

[Daniel Bates]

(In reply to comment #7)
>[...] From my understanding of this changeset and the current incarnation of this source code and its call sites, it seems that the use of m_mouseDownWasInSubframe is related to some idiosyncrasies with NSView.

I should also remark that we should further investigate whether this instance variable is still needed. I suggest that we do this in a separate bug.

[Maciej Stachowiak]

Comment on attachment 88545 [details]
Patch and layout test

I think this is probably the right thing to do for most cases, but in the splitter case that was the genesis of this bug, I am pretty sure sending a mouseup at the end of splitter dragging can only possibly have bad effects even if other browsers do it too.

Otherwise the code looks fine.

I guess you would also need to update the test and maybe make a separate test for dragging a non-frame thing other than the splitter.

[Maciej Stachowiak]

For instance, dragging from the main frame to a subframe should probably send the mouseup for web compat reasons.