314350 - implement nsIScrollable::GetScrolledSize()

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Håkan Waara]

I kind of "accidently" implemented this in a proposed fix to bug 222274.

Comment 1

[Håkan Waara]

Attachment: patch

I don't know who to CC for reviews...?

Comment 2

[Martijn Wargers (dead)]

Well, you can look at the changelog of the file:
http://bonsai.mozilla.org/cvslog.cgi?file=mozilla/layout/xul/base/src/nsScrollBoxObject.cpp
So I think you could ask bzbarsky, roc, bryner or dbaron.

Comment 3

[Håkan Waara]

Comment on attachment 201290 [details] [diff] [review]
patch

Pretty straightforward patch. Look OK to you, bz?

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 201290 [details] [diff] [review]
patch

>Index: layout/xul/base/src/nsScrollBoxObject.cpp
>+    nsIFrame* scrolledBox = GetScrolledBox (this);

Why the space before '('?

>+    *width 	= NSToIntRound (scrollRect.width * twipsToPixels);

Why not use NSTwipsToIntPixels instead?  And again, what's with the space before '('?

Comment 5

[Håkan Waara]

(In reply to comment #4)
> (From update of attachment 201290 [details] [diff] [review] [edit])
> >Index: layout/xul/base/src/nsScrollBoxObject.cpp
> >+    nsIFrame* scrolledBox = GetScrolledBox (this);
> Why the space before '('?

Well, it's just my coding style, I think it's easier to read that way.

> >+    *width 	= NSToIntRound (scrollRect.width * twipsToPixels);
> Why not use NSTwipsToIntPixels instead?  And again, what's with the space
> before '('?

I'll switch to using that function instead.

Comment 6

[Boris Zbarsky [:bzbarsky]]

> Well, it's just my coding style, I think it's easier to read that way.

Please use prevailing style in the file -- no space there.

Comment 7

[Håkan Waara]

Attachment: patch v2

Better patch addressing bz's comments.

Comment 8

[Håkan Waara]

fix checked in.

Comment 9

[Asaf Romano (gone)]

Asking for blocking1.8.1, we need this fix for bug 342841.

Comment 10

[Mike Connor [:mconnor]]

Comment on attachment 201362 [details] [diff] [review]
patch v2

a=mconnor on behalf of drivers, straightforward patch that's needed for a blocker,

Comment 11

[Håkan Waara]

I don't have a branch tree handy, so if anyone wants to land this for me, that'd be great.

Comment 12

[Asaf Romano (gone)]

Checking in nsScrollBoxObject.cpp;
/cvsroot/mozilla/layout/xul/base/src/nsScrollBoxObject.cpp,v  <--  nsScrollBoxObject.cpp
new revision: 1.21.12.2; previous revision: 1.21.12.1
done