Bug 40981 – iframe and scrollbar with "overflow:auto" should support autoscroll with mousedrag

• Status: RESOLVED
• Resolution: FIXED
• Component: Frames

[Hajime Morrita]

Not only for outermost frame, but also iframe, should support autoscroll with mousedrag

How to reproduce:
1. Open reproduction HTML.
2. Drag "Drag me" to bottom of the iframe.

Expected:
- The content of iframe start scrolling down 
Actual:
- nothing did happen.

This bug is drived from Bug 39725.

[Erik Arvidsson]

There is a misconception here. Bug 39725 was about elements but was treated as top level browser viewport. The test case for bug 39725 uses a div which that bug was about. To keep things clear, I'll add an iframe test for this one.

[Erik Arvidsson]

Created attachment 59386 [details]
Test case with iframe

[Hajime Morrita]

>There is a misconception here. Bug 39725 was about elements but was treated as top level browser viewport. The test case for bug 39725 uses a div which that bug was about. To keep things clear, I'll add an iframe test for this one.
Oops, you are right. Thank you for pointing it out!

[Antonio Gomes]

Taking. Tested on:

Safari / Chrome / Opera15 - No autoscroll
Firefox - No autoscroll
Opera 12 (pre-blink) - Autoscrolls

[Antonio Gomes]

So, I looked at this, and there seems to be two main issues here:

1) ChromeClient::shouldAutoscrollForDragAndDrop returns  'false' unconditionally:

    // FIXME: Port should return true using heuristic based on scrollable(RenderBox).
    virtual bool shouldAutoscrollForDragAndDrop(RenderBox*) const { return false; }

2) Within DragController, drag'ing content from a Frame (main) to another (inner) is "blocked" by the SecurityOrigin policies:

bool DragController::tryDocumentDrag(DragData* dragData, DragDestinationAction actionMask, DragSession& dragSession)
{
...
    if (m_dragInitiator && !m_documentUnderMouse->securityOrigin()->canReceiveDragData(m_dragInitiator->securityOrigin()))
        return false;



The one that actually concerns me is (2). Do you think we can have a way to implement this feature without breaking the SecurityOrigin contract?

[Antonio Gomes]

Relevant changeset: http://trac.webkit.org/changeset/71925

    2010-11-11 Abhishek Arya <​[e-mail]>

        Reviewed by Adam Barth.

        Not allow drag and drop across different origins.
        ​https://bugs.webkit.org/show_bug.cgi?id=49098

[Antonio Gomes]

So, is this behavior something Safari wants?

I have a patch for this ready, but the below ChromeClient hook would have to return true for Mac, and I would like to hear from you guys.

    virtual bool shouldAutoscrollForDragAndDrop(RenderBox*) const { return false; }

For the record, Chrome/Opera/IE support autoscroll for scrollable divs and iframes. Firefox does not.

Please advice.

[Simon Fraser (smfr)]

(In reply to comment #7)
> So, is this behavior something Safari wants?
> 
> I have a patch for this ready, but the below ChromeClient hook would have to return true for Mac, and I would like to hear from you guys.

Let's go with "no" for now to not change current behavior.

[Antonio Gomes]

(In reply to comment #8)
> (In reply to comment #7)
> > So, is this behavior something Safari wants?
> > 
> > I have a patch for this ready, but the below ChromeClient hook would have to return true for Mac, and I would like to hear from you guys.
> 
> Let's go with "no" for now to not change current behavior.

Sounds ok.

Would it be less aggressive better:

For example, allow autoscroll to scroll parent scrollable div's and/or non-mainframe frames? This would match autoscroll from text-selection..

Otherwise, we could instead start removing code, if it is a not desirable feature.

I appreciate the input.

[Antonio Gomes]

(In reply to comment #8)
> (In reply to comment #7)
> > So, is this behavior something Safari wants?
> > 
> > I have a patch for this ready, but the below ChromeClient hook would have to return true for Mac, and I would like to hear from you guys.
> 
> Let's go with "no" for now to not change current behavior.

CC'ing Carlos/Gustavo, Allan and Gyuyoung for Gtk/Qt/Efl respectively.

Guys, is this something you think "your" ports are interested? If so, I can work for that. Otherwise, it will be dead code.

[Antonio Gomes]

Created attachment 211839 [details]
experiment for EWS

[Gustavo Noronha (kov)]

I believe this is good to have in webkitgtk, this is indeed how things work on most gtk widgets.

[Antonio Gomes]

Created attachment 212012 [details]
patch

QtWebKit and WebKitGtk+ are interested in this feature. As per, discussion on IRC/Bug.

[Antonio Gomes]

Kindly ping review.

[Antonio Gomes]

Fixed by https://trac.webkit.org/r156257

[WebKit Commit Bot]

Re-opened since this is blocked by bug 121777

[Alexey Proskuryakov]

The test in this patch fails: <http://build.webkit.org/results/Apple%20MountainLion%20Release%20WK2%20(Tests)/r156257%20(11977)/fast/events/drag-and-drop-autoscroll-inner-frame-pretty-diff.html>.

Antonio is not available at the moment, and there are two more unrelated regressions in the tree. Fixing everything is not an option for me, rolling out.

[Alexey Proskuryakov]

Rolled out in <http://trac.webkit.org/changeset/156259>.

[Antonio Gomes]

(In reply to comment #17)
> The test in this patch fails: <http://build.webkit.org/results/Apple%20MountainLion%20Release%20WK2%20(Tests)/r156257%20(11977)/fast/events/drag-and-drop-autoscroll-inner-frame-pretty-diff.html>.
> 
> Antonio is not available at the moment, and there are two more unrelated regressions in the tree. Fixing everything is not an option for me, rolling out.

Thanks.

EWS betrayed me, and it also passed locally. Will investigate..

[Antonio Gomes]

Created attachment 212390 [details]
patch - with tighten test

[WebKit Commit Bot]

Comment on attachment 212390 [details]
patch - with tighten test

Clearing flags on attachment: 212390

Committed r156297: <http://trac.webkit.org/changeset/156297>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.