Bug 73592 - REGRESSION (r100483): Can’t drag out of background window
Summary: REGRESSION (r100483): Can’t drag out of background window
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: UI Events (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Beth Dakin
URL:
Keywords: InRadar, Regression
Depends on:
Blocks:
 
Reported: 2011-12-01 14:54 PST by Beth Dakin
Modified: 2011-12-01 15:33 PST (History)
1 user (show)

See Also:


Attachments
Patch (3.05 KB, patch)
2011-12-01 14:58 PST, Beth Dakin
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Beth Dakin 2011-12-01 14:54:57 PST
* 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>
Comment 1 Beth Dakin 2011-12-01 14:58:28 PST
Created attachment 117496 [details]
Patch
Comment 2 Darin Adler 2011-12-01 15:04:23 PST
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".
Comment 3 Beth Dakin 2011-12-01 15:29:14 PST
(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.
Comment 4 Beth Dakin 2011-12-01 15:33:01 PST
Committed change with revision 101719.