Bug 84873 – Make ScrollView::scrollSize scrollbar-independent

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Antonio Gomes]

This method is so far tied to the existence of scrollbars to identify a scrollview as scrollable.

for ports that disable scrollbars at FrameView creation it makes it impossible to use any ScrollAnimatorXXX method.

[Antonio Gomes]

Created attachment 138840 [details]
(r115345 + r115401, r=andersca) patch v1

[Adam Treat]

Why have the conditional at all?  Why not just calculate in your fashion irregardless of whether there are scrollbars or not?

[Antonio Gomes]

(In reply to comment #2)
> Why have the conditional at all?  Why not just calculate in your fashion irregardless of whether there are scrollbars or not?

Fair enough.

[Antonio Gomes]

Created attachment 138927 [details]
patch v2 - implementing what Adam questioned

[Adam Treat]

(In reply to comment #4)
> Created an attachment (id=138927) [details]
> patch v2 - implementing what Adam questioned

LGTM, but since I haven't touched this code in a long time (at least not in upstream repo) I think someone else should review too.  Thanks Antonio!

[Antonio Gomes]

Comment on attachment 138840 [details]
(r115345 + r115401, r=andersca) patch v1

Committed: <http://trac.webkit.org/changeset/115345>

[Darin Adler]

It seems that http://trac.webkit.org/changeset/115401 went in without any comments here explaining why it’s needed.

[Antonio Gomes]

Ooh my :(

I did a bad job at first (pushed the wrong patch) and at second (amended the proper patch as a diff against what was committed originally) without a changelog entry :/

Darin, want me to revert both, and push it properly?

[Darin Adler]

I’m not sure there is any significant benefit to trying to fix this now; I just would have liked to have a comment here in the bug explaining what happened.