Closed Bug 503814 Opened 11 years ago Closed 11 years ago

iframe contents shifted slightly due to rounding issue


(Core :: Layout, defect)

Windows XP
Not set





(Reporter: tnikkel, Assigned: tnikkel)




(2 files)

Attached patch patchSplinter Review
The patch in bug 448830 replaced ScaleRoundPreservingCentersInverse with the seemingly functionally equivalent ToNearestPixels. Except ToNearestPixels uses NSToIntRound where ScaleRoundPreservingCentersInverse used NSToCoordRound. NSToIntRound rounds away from zero, whereas NSToCoordRound rounds towards positive infinity. Robert's blog describes the problem well . In this specific case nsView::CalcWidgetBounds takes the viewBounds with y = -30 and height = 9000 and gets pixel bounds with y = -1 and height = 151, which should be y = 0 and height = 150.

I did that safe thing and didn't use the fast NS_lroundup30 from NSToCoordRound since we are dealing with ints that may not be restricted to nscoord values.

I think bug 477236 is the same issue, but I can't reproduce so I can't tell.

I did a quick survey of the other users of NSToIntRound, it seems like a lot of them should be using NSToIntRoundUp. We should audit all uses of NSToIntRound as there is at least one similar bug (bug 428446) that predates the landing from bug 448830 that might be fixed by always rounding up.
Attachment #388176 - Flags: superreview?(roc)
Attachment #388176 - Flags: review?(roc)
Attached file testcase
Looks fine on Linux, but on Windows you can see the problem. The background colors aren't needed, but they make the problem easier to see.
Comment on attachment 388176 [details] [diff] [review]

Mmmm ... thanks!
Attachment #388176 - Flags: superreview?(roc)
Attachment #388176 - Flags: superreview+
Attachment #388176 - Flags: review?(roc)
Attachment #388176 - Flags: review+
Keywords: checkin-needed
Whiteboard: [needs landing]
Flags: blocking1.9.2?
Closed: 11 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Keywords: checkin-needed
Whiteboard: [needs landing]
Flags: blocking1.9.2? → wanted1.9.2+
Depends on: 600148
You need to log in before you can comment on or make changes to this bug.