WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
40461
[Chromium] Dragging outside the frame immediately causes page to scroll
https://bugs.webkit.org/show_bug.cgi?id=40461
Summary
[Chromium] Dragging outside the frame immediately causes page to scroll
Hajime Morrita
Reported
2010-06-10 22:09:27 PDT
Originated from: *
http://code.google.com/p/chromium/issues/detail?id=45900
Attachments
patch v0
(9.64 KB, patch)
2010-06-10 22:21 PDT
,
Hajime Morrita
no flags
Details
Formatted Diff
Diff
patch v1
(9.62 KB, patch)
2010-06-16 04:06 PDT
,
Hajime Morrita
no flags
Details
Formatted Diff
Diff
patch v2
(14.13 KB, patch)
2010-06-17 00:02 PDT
,
Hajime Morrita
no flags
Details
Formatted Diff
Diff
patch v3
(14.13 KB, patch)
2010-06-17 18:15 PDT
,
Hajime Morrita
fishd
: review+
fishd
: commit-queue-
Details
Formatted Diff
Diff
Show Obsolete
(3)
View All
Add attachment
proposed patch, testcase, etc.
Hajime Morrita
Comment 1
2010-06-10 22:21:07 PDT
Created
attachment 58446
[details]
patch v0
Tony Chang
Comment 2
2010-06-11 00:10:54 PDT
Comment on
attachment 58446
[details]
patch v0 Looks OK to me, but I am not a reviewer. I guess it's hard to write a layout test for this since it has a timer. ---------------------------------
http://wkrietveld.appspot.com/40461/diff/1/3
File WebKit/chromium/src/WebViewImpl.cpp (right):
http://wkrietveld.appspot.com/40461/diff/1/3#newcode236
WebKit/chromium/src/WebViewImpl.cpp:236: if (shouldScroll() && !m_timer.isActive()) Nit: Just make this an else if?
Hajime Morrita
Comment 3
2010-06-16 04:06:14 PDT
Created
attachment 58871
[details]
patch v1
Hajime Morrita
Comment 4
2010-06-16 04:07:47 PDT
Hi Tony, thank you for reviewing!
> Looks OK to me, but I am not a reviewer. I guess it's hard to write a layout test for this since it has a timer.
Ya, timer and drag-n-drop make automated tests harder...
>
http://wkrietveld.appspot.com/40461/diff/1/3#newcode236
> WebKit/chromium/src/WebViewImpl.cpp:236: if (shouldScroll() && !m_timer.isActive()) > Nit: Just make this an else if?
Done.
Darin Fisher (:fishd, Google)
Comment 5
2010-06-16 11:25:40 PDT
Comment on
attachment 58871
[details]
patch v1 WebKit/chromium/src/WebViewImpl.cpp:167 + class WebDragScrollTimer { this should get its own .h,.cpp files. it should not be prefixed with Web since it is not part of the WebKit API. DragScrollTimer should be fine.
Hajime Morrita
Comment 6
2010-06-17 00:02:00 PDT
Created
attachment 58965
[details]
patch v2
Hajime Morrita
Comment 7
2010-06-17 00:05:13 PDT
Hi Darin, thank you for quick response! (In reply to
comment #5
)
> (From update of
attachment 58871
[details]
) > WebKit/chromium/src/WebViewImpl.cpp:167 > + class WebDragScrollTimer { > this should get its own .h,.cpp files. it should not be prefixed > with Web since it is not part of the WebKit API. DragScrollTimer > should be fine.
Fixed to move to DragScrollTimer to separate source files.
WebKit Review Bot
Comment 8
2010-06-17 00:05:34 PDT
Attachment 58965
[details]
did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1 WebKit/chromium/src/DragScrollTimer.h:36: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 1 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style.
WebKit Review Bot
Comment 9
2010-06-17 00:22:41 PDT
Attachment 58965
[details]
did not build on chromium: Build output:
http://webkit-commit-queue.appspot.com/results/3314273
Hajime Morrita
Comment 10
2010-06-17 18:15:57 PDT
Created
attachment 59056
[details]
patch v3
WebKit Review Bot
Comment 11
2010-06-17 19:28:58 PDT
Attachment 59056
[details]
did not build on chromium: Build output:
http://webkit-commit-queue.appspot.com/results/3330281
Hajime Morrita
Comment 12
2010-06-17 22:09:03 PDT
(In reply to
comment #11
)
>
Attachment 59056
[details]
did not build on chromium: > Build output:
http://webkit-commit-queue.appspot.com/results/3330281
Hmm. buildbot cannot detect files addition...
Dimitri Glazkov (Google)
Comment 13
2010-06-17 22:14:26 PDT
(In reply to
comment #12
)
> (In reply to
comment #11
) > >
Attachment 59056
[details]
[details] did not build on chromium: > > Build output:
http://webkit-commit-queue.appspot.com/results/3330281
> > Hmm. buildbot cannot detect files addition...
Nope. We should fix that :)
Darin Fisher (:fishd, Google)
Comment 14
2010-06-18 13:38:05 PDT
Comment on
attachment 59056
[details]
patch v3 WebKit/chromium/src/DragScrollTimer.h:67 + nit: extra new line
Hajime Morrita
Comment 15
2010-06-21 19:29:25 PDT
Committed
r61598
: <
http://trac.webkit.org/changeset/61598
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug