Bug 136650 - Rubber-banding in the DOM view of the WebInspector is really jumpy
Summary: Rubber-banding in the DOM view of the WebInspector is really jumpy
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2014-09-08 17:17 PDT by Beth Dakin
Modified: 2014-11-10 20:36 PST (History)
10 users (show)

See Also:


Attachments
Reduction (2.24 KB, text/html)
2014-09-08 17:18 PDT, Beth Dakin
no flags Details
Patch (2.93 KB, patch)
2014-09-08 17:21 PDT, Beth Dakin
simon.fraser: review+
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2 (1.42 MB, application/zip)
2014-09-08 18:14 PDT, Build Bot
no flags Details
Patch (2.93 KB, patch)
2014-09-09 15:50 PDT, 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 2014-09-08 17:17:23 PDT
* SUMMARY
If you try to rubber-band in the DOM view of the WebInspector, things get a bit jumpy.

* STEPS TO REPRODUCE
1. Open a web page, I used http://www.webkit.org.
2. Open the WebInspector and make sure the DOM tree view is visible.
3. Make sure the DOM tree view is scrollable by expanding as many of the nodes as you can (I option-clicked the body).
4. Scroll and rubber-band the view with your mouse cursor over the nodes (you will notice the blue hightlight moving from node to node staying under your cursor).

* RESULTS
As you rubber-band, the view will jump around a bit.

* REGRESSION
This is new with the changes to add rubber-banding in overflow areas.

* NOTES
If you rubber-band over the scrollbar track while it is visible or when the WebInspector is not the frontmost window, you will not see the jumping, but you will notice the blue hightlight also does not change.

rdar://problem/18166043
Comment 1 Beth Dakin 2014-09-08 17:18:07 PDT
Created attachment 237821 [details]
Reduction
Comment 2 Beth Dakin 2014-09-08 17:21:27 PDT
Created attachment 237822 [details]
Patch
Comment 3 Simon Fraser (smfr) 2014-09-08 17:30:29 PDT
Comment on attachment 237822 [details]
Patch

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

> Source/WebCore/rendering/RenderLayer.cpp:1390
> +    if (!renderer().frame().settings().rubberBandingForOverflowScrollEnabled())
> +        return false;

Do you need to check the setting here, since presumably overhangAmount() will just return 0,0 otherwise?
Comment 4 Beth Dakin 2014-09-08 17:35:45 PDT
(In reply to comment #3)
> (From update of attachment 237822 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=237822&action=review
> 
> > Source/WebCore/rendering/RenderLayer.cpp:1390
> > +    if (!renderer().frame().settings().rubberBandingForOverflowScrollEnabled())
> > +        return false;
> 
> Do you need to check the setting here, since presumably overhangAmount() will just return 0,0 otherwise?

True. Probably no need to belt-and-suspenders here.
Comment 5 Build Bot 2014-09-08 18:14:20 PDT
Comment on attachment 237822 [details]
Patch

Attachment 237822 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/6264184811552768

New failing tests:
css3/flexbox/flexbox-overflow-auto.html
css3/flexbox/flexbox-baseline.html
css3/flexbox/child-overflow.html
Comment 6 Build Bot 2014-09-08 18:14:23 PDT
Created attachment 237826 [details]
Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-14  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 7 Beth Dakin 2014-09-09 15:50:00 PDT
Created attachment 237865 [details]
Patch

Here's a new patch that shouldn't break any tests or other ports. RenderLayer should call into ScrollAnimatorMac's implementation of isRubberBandInProgress() that will check to ensure there is a non-zero overhangAmount (like my first patch did) but it will also make sure you are in the middle of a momentum gesture.
Comment 8 Darin Adler 2014-09-10 08:32:04 PDT
Comment on attachment 237865 [details]
Patch

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

> Source/WebCore/rendering/RenderLayer.cpp:1399
> +    return false;
> +#else
> +    return false;
> +#endif

Could omit the else and share the "return false". Not sure if it would be better or not.
Comment 9 Beth Dakin 2014-09-10 13:41:15 PDT
http://trac.webkit.org/changeset/173484
Comment 10 Beth Dakin 2014-09-10 13:42:11 PDT
Thanks Darin!
Comment 11 Simon Fraser (smfr) 2014-11-10 20:36:55 PST
This caused bug 138525.