Bug 73592 – REGRESSION (r100483): Can’t drag out of background window

• Component: UI Events
• Status: RESOLVED FIXED

[Beth Dakin]

* SUMMARY
In recent builds trying to Command-drag a link or selected text from a background window fails.

* STEPS TO REPRODUCE
1. Navigate to a webpage with links
2. Move Safari to the background
3. Command-drag a link out of the Safari window

* RESULTS
The link gets into the active state while being dragged, but isn’t dragged out. Similarly, selected text cannot be command-dragged.

This is a regression from revision 100483.

<rdar://problem/10508870>

[Beth Dakin]

Created attachment 117496 [details]
Patch

[Darin Adler]

Comment on attachment 117496 [details]
Patch 

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

Is there a good way to regression-test this?

> Source/WebKit2/ChangeLog:4
> +        REGRESSION (r100483): Canât drag out of background window

Character encoding trouble here with the ’ character.

> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:1188
> +        // We need to do a full, normal hit test during this mouse event if the page is active or if a mouse
> +        // button is currently pressed. It is possible that neither of those things will be true since on 
> +        // Lion when legacy scrollbars are enabled, WebKit receives mouse events all the time. If it is one 
> +        // of those cases where the page is not active and the mouse is not pressed, then we can fire a more
> +        // efficient scrollbars-only version of the event.
> +        bool mouseIsPressed = mouseEvent.button() != WebMouseEvent::NoButton;
> +        handled = handleMouseEvent(mouseEvent, m_page.get(), !(m_page->focusController()->isActive() || mouseIsPressed));

To make the code match the comment it would be better if the third argument was a named local variable called something like "useMoreEfficientScrollbarsOnlyVersion".

[Beth Dakin]

(In reply to comment #2)
> (From update of attachment 117496 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=117496&action=review
> 
> Is there a good way to regression-test this?
> 

I'm not sure there is. Our testing tools typically claim to be "key" windows all the time, so just getting a window to be inactive is not something we can replicate in a test (as far as I know) without enhancing the testing tool. Maybe that is worth doing actually, but then as far as I know there is also not a good way to test link dragging (but I am double-checking with Enrica on that). There are a number of unfortunate stumbling blocks here.

> > Source/WebKit2/ChangeLog:4
> > +        REGRESSION (r100483): Canât drag out of background window
> 
> Character encoding trouble here with the ’ character.
> 

Oops! Will fix.

> > Source/WebKit2/WebProcess/WebPage/WebPage.cpp:1188
> > +        // We need to do a full, normal hit test during this mouse event if the page is active or if a mouse
> > +        // button is currently pressed. It is possible that neither of those things will be true since on 
> > +        // Lion when legacy scrollbars are enabled, WebKit receives mouse events all the time. If it is one 
> > +        // of those cases where the page is not active and the mouse is not pressed, then we can fire a more
> > +        // efficient scrollbars-only version of the event.
> > +        bool mouseIsPressed = mouseEvent.button() != WebMouseEvent::NoButton;
> > +        handled = handleMouseEvent(mouseEvent, m_page.get(), !(m_page->focusController()->isActive() || mouseIsPressed));
> 
> To make the code match the comment it would be better if the third argument was a named local variable called something like "useMoreEfficientScrollbarsOnlyVersion".

Sounds good.

[Beth Dakin]

Committed change with revision 101719.