ScrollView::platformVisibleContentRect returns incorrectly sized rectangle when includeScrollbars == true

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Daniel Bates]

For the Apple Mac port, the method ScrollView::platformVisibleContentRect returns an incorrectly sized rectangle when includeScrollbars == true.

Moreover, the dimensions of this rectangle disagree with both the comment in file ScrollView.h <http://trac.webkit.org/browser/trunk/WebCore/platform/ScrollView.h?rev=53259#L112> and the rectangle that would be returned by the method ScrollView::visibleContentRect <http://trac.webkit.org/browser/trunk/WebCore/platform/ScrollView.cpp?rev=53259#L182> had the widget not been platform-specific (i.e. platformWidget() == false).

[Daniel Bates]

From my understanding of the hierarchy of a NSScrollView (*), when includeScrollbars == true, ScrollView::platformVisibleContentRect returns the rectangle of the document within the content view of the scroll view. And this rectangle has the same size as the rectangle returned by -[NSScrollView documentVisibleRect] (i.e. the rectangle returned when includeScrollbars == false).

(*) See subsection Components of a Scroll View of section How Scroll Views Work of the Scroll View Programming Guide for Cocoa <http://developer.apple.com/mac/library/documentation/Cocoa/Conceptual/NSScrollViewGuide/Articles/Introduction.html#//apple_ref/doc/uid/TP40003460>

[Daniel Bates]

Created attachment 46931 [details]
Patch

No test included, since I cannot think of a way to test this change using either a manual or DRT test.

[Darin Adler]

(In reply to comment #2)
> No test included, since I cannot think of a way to test this change using
> either a manual or DRT test.

If this has no effect on browser behavior, how did you discover the problem?

[Daniel Bates]

(In reply to comment #3)
> (In reply to comment #2)
> > No test included, since I cannot think of a way to test this change using
> > either a manual or DRT test.
> 
> If this has no effect on browser behavior, how did you discover the problem?

In looking into another bug involving scrolling I noticed that the result of ScrollView::visibleContentRect was a rectangle whose dimensions did not include scrollbar even when I passed the parameter includeScrollbars == true. Upon inspection and from my understanding of the API of NSScrollView, NSClipView, NSView, and the description in Scroll View Programming Guide for Cocoa, it seems that [[NSScrollView documentView] visibleRect] == [[NSScrollView documentVisibleRect].

[Darin Adler]

Comment on attachment 46931 [details]
Patch

rs=me

Seems safe to do this since it currently has no effect. Later it might come in handy to have a correct implementation.

[Daniel Bates]

Comment on attachment 46931 [details]
Patch

Clearing flags on attachment: 46931

Committed r53490: <http://trac.webkit.org/changeset/53490>

[Daniel Bates]

All reviewed patches have been landed.  Closing bug.

[Simon Fraser (smfr)]

This broke autoscrolling in Mail on Mac. -documentVisibleRect is not equivalent to -visibleRect, because the latter takes all enclosing views into account. This is required to make drag-scrolling work.

[Darin Adler]

Darn, that's not good! Can we just roll this out?

[Daniel Bates]

Spoke to Simon Fraser on IRC about this. There is still an issue with the returned dimensions. Simon mentioned that he will look to fix the issue while preventing the regression. I am also looking into this issue. If needed, feel free to rollout this patch.

(In reply to comment #9)
> Darn, that's not good! Can we just roll this out?