Bug 40981

Summary: iframe and scrollbar with "overflow:auto" should support autoscroll with mousedrag
Product: WebKit Reporter: Hajime Morrita <morrita>
Component: FramesAssignee: Antonio Gomes <tonikitoo>
Status: RESOLVED FIXED    
Severity: Normal CC: adele, allan.jensen, arv, bdakin, cgarcia, commit-queue, darin, esprehn+autocc, glenn, gustavo, gyuyoung.kim, kondapallykalyan, simon.fraser
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Bug Depends on: 121559, 121777    
Bug Blocks:    
Attachments:
Description Flags
Test case with iframe
none
experiment for EWS
none
patch
none
patch - with tighten test none

Hajime Morrita
Reported 2010-06-22 04:38:06 PDT
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.
Attachments
Test case with iframe (271 bytes, text/html)
2010-06-22 10:19 PDT, Erik Arvidsson 		no flags
Details
experiment for EWS (9.32 KB, patch)
2013-09-16 16:01 PDT, Antonio Gomes 		no flags
DetailsFormatted DiffDiff
patch (9.80 KB, patch)
2013-09-18 12:26 PDT, Antonio Gomes 		no flags
DetailsFormatted DiffDiff
patch - with tighten test (10.45 KB, patch)
2013-09-23 14:15 PDT, Antonio Gomes 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Erik Arvidsson
Comment 1 2010-06-22 10:12:21 PDT
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
Comment 2 2010-06-22 10:19:10 PDT
Created attachment 59386 [details] Test case with iframe
Hajime Morrita
Comment 3 2010-06-22 17:43:45 PDT
>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
Comment 4 2013-08-30 11:02:34 PDT
Taking. Tested on: Safari / Chrome / Opera15 - No autoscroll Firefox - No autoscroll Opera 12 (pre-blink) - Autoscrolls
Antonio Gomes
Comment 5 2013-09-03 05:18:31 PDT
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
Comment 6 2013-09-03 05:31:03 PDT
Relevant changeset: http://trac.webkit.org/changeset/71925 2010-11-11 Abhishek Arya <​inferno@chromium.org> Reviewed by Adam Barth. Not allow drag and drop across different origins. ​https://bugs.webkit.org/show_bug.cgi?id=49098
Antonio Gomes
Comment 7 2013-09-16 13:59:09 PDT
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)
Comment 8 2013-09-16 14:10:35 PDT
(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
Comment 9 2013-09-16 14:22:37 PDT
(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
Comment 10 2013-09-16 15:58:14 PDT
(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
Comment 11 2013-09-16 16:01:56 PDT
Created attachment 211839 [details] experiment for EWS
Gustavo Noronha (kov)
Comment 12 2013-09-17 07:28:41 PDT
I believe this is good to have in webkitgtk, this is indeed how things work on most gtk widgets.
Antonio Gomes
Comment 13 2013-09-18 12:26:39 PDT
Created attachment 212012 [details] patch QtWebKit and WebKitGtk+ are interested in this feature. As per, discussion on IRC/Bug.
Antonio Gomes
Comment 14 2013-09-20 13:47:10 PDT
Kindly ping review.
Antonio Gomes
Comment 15 2013-09-22 21:50:35 PDT
Fixed by https://trac.webkit.org/r156257
WebKit Commit Bot
Comment 16 2013-09-22 23:23:43 PDT
Re-opened since this is blocked by bug 121777
Alexey Proskuryakov
Comment 17 2013-09-22 23:28:39 PDT
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
Comment 18 2013-09-22 23:32:32 PDT
Rolled out in <http://trac.webkit.org/changeset/156259>.
Antonio Gomes
Comment 19 2013-09-23 05:46:01 PDT
(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
Comment 20 2013-09-23 14:15:36 PDT
Created attachment 212390 [details] patch - with tighten test
WebKit Commit Bot
Comment 21 2013-09-23 14:49:08 PDT
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
Comment 22 2013-09-23 14:49:14 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.