implement nsIScrollable::GetScrolledSize()

RESOLVED FIXED

Status

()

Core
Layout
RESOLVED FIXED
12 years ago
12 years ago

People

(Reporter: Håkan Waara, Assigned: Håkan Waara)

Tracking

({fixed1.8.1})

Trunk
x86
Windows XP
fixed1.8.1
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

12 years ago
I kind of "accidently" implemented this in a proposed fix to bug 222274.
(Assignee)

Comment 1

12 years ago
Created attachment 201290 [details] [diff] [review]
patch

I don't know who to CC for reviews...?
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.
(Assignee)

Comment 3

12 years ago
Comment on attachment 201290 [details] [diff] [review]
patch

Pretty straightforward patch. Look OK to you, bz?
Attachment #201290 - Flags: review?(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 '('?
(Assignee)

Comment 5

12 years ago
(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.
> 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.
(Assignee)

Comment 7

12 years ago
Created attachment 201362 [details] [diff] [review]
patch v2

Better patch addressing bz's comments.
Attachment #201290 - Attachment is obsolete: true
Attachment #201290 - Flags: review?(bzbarsky)
Attachment #201362 - Flags: superreview+
Attachment #201362 - Flags: review+
(Assignee)

Comment 8

12 years ago
fix checked in.
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
Asking for blocking1.8.1, we need this fix for bug 342841.
Blocks: 342841
Flags: blocking1.8.1?
Attachment #201362 - Flags: approval1.8.1?
Comment on attachment 201362 [details] [diff] [review]
patch v2

a=mconnor on behalf of drivers, straightforward patch that's needed for a blocker,
Attachment #201362 - Flags: approval1.8.1? → approval1.8.1+
(Assignee)

Comment 11

12 years ago
I don't have a branch tree handy, so if anyone wants to land this for me, that'd be great.
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
Keywords: fixed1.8.1
Flags: blocking1.8.1?
You need to log in before you can comment on or make changes to this bug.