Closed
Bug 7032
Opened 25 years ago
Closed 25 years ago
incorrect nsRect usages
Categories
(Core :: Layout: Form Controls, defect, P3)
Tracking
()
VERIFIED
FIXED
M9
People
(Reporter: stuartp, Assigned: kmcclusk)
Details
(Whiteboard: Have fix, will checkin as soon as tree as green.)
The use of nsRect in the majority of the codebase, from what I have seen, is
invalid. It stores widths and heights as signed ints, whereas all uses of these
are unsigned ints. This conversion is causing many calls to various things,
such as nsWidget::Resize() to have extremely large sizes at times. The last
time I looked at this, it seemed to be happening a great deal in nsView* where
it was subtracting out the scrollbar dimensions. this was causing negative
widths (instead of 0), which were then being passed to the widget code as
unsigned ints, converting them to rather huge numbers.
I don't see any purpose in nsRect's widths and heights being signed. Is there a
reason for this?
Updated•25 years ago
|
Target Milestone: M7
Updated•25 years ago
|
Assignee: peterl → karnaze
Component: Layout → Widget Set
Comment 2•25 years ago
|
||
The error here that I see is in the widget code.
nsRect uses nscoord for all of its values, not int, not unsigned int. Ignore the
fact that nscoord is *currently* typedefed to an int, nscoord is a *signed
cordinate value* nothing more, nothing less. nscoord and ints are NOT
interchangeable.
For the record, it's also perfectly legal to have a negative width or height in
a rect. It's an interesting mathematical state, not an invalid one.
The widget APIs and code should be using nscoord for its coordinate values, not
ints. Nowhere in the codebase should we be interchanging nscoords and ints
except when converting for native APIs or from *other* int values, like number
of pixels. And those cases should be using sanctioned conversion functions to go
from nscoord space into native or pixel space. (see nsUnitConversion for many
examples...)
Updated•25 years ago
|
Assignee: karnaze → kmcclusk
Comment 3•25 years ago
|
||
Kevin, please make sure that we are using nscoords as Peter says.
Assignee | ||
Updated•25 years ago
|
Target Milestone: M7 → M8
Assignee | ||
Updated•25 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Updated•25 years ago
|
Target Milestone: M8 → M9
Moving all Widget Set bugs, past and present, to new HTML Form Controls
component per request from karnaze. Widget Set component will be retired
shortly.
Assignee | ||
Comment 7•25 years ago
|
||
The widget library works in device units, not app units as implied by nscoord.
Changing widget interface to accept nscoord values would require the widget
library to understand how to convert app units into device units.
The solution is to leave nsWidget accepting device units, but change any xpcode
which passes bad values to nsWidget. We should also change all of the
nsIWidget's usage of PRUint32's to PRInt32. This will not fix the bad usage it
will simply allow us to put assertions in the platform specific widget code so
we can detect when a bad call is made to a nsIWidget (i.e a negative value is
passed for width or height). Restricting the device units to unsigned values is
not desirable anyway. We don't need the extra positive extent of unsigned int's
since the underlying window environments usually can not accept these large
values anyway. I will change nsIWidget to use PRInt32's instead of PRUint32's
when refering to device coordinates. I will also add assertions on WIN32 to flag
places where negative values are passed for width's and heights.
Assignee | ||
Updated•25 years ago
|
Assignee: kmcclusk → beard
Status: ASSIGNED → NEW
Assignee | ||
Comment 8•25 years ago
|
||
Changed nsIWidget::Resize(...), nsIWidget::Move(...) to use PRInt32 instead of
PRUInt32. Added assertions to nsWindow::Resize and nsWindow::Move under WIN32 to
halt when a negative width or height is passed.
Modified Mac and Linux to match.
Patrick, I'm re-assigning this bug to you, Can you keep and eye on occurences
when a negative width or height is passed to nsIWidget::Resize or
nsIWIdget::Remove from the view manager? If this happens under WIN32 an assert
will be triggered. I ran the viewer on a number of pages (test0..15) 20 of the
top 100 and never saw an assert.
Updated•25 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 9•25 years ago
|
||
I found and corrected a number of places in views, and viewer test app where
negative values where being passed to nsIWidget::Resize. I have corrected them,
and I will check them in when the tree opens.
Updated•25 years ago
|
Assignee: beard → kmcclusk
Status: ASSIGNED → NEW
Assignee | ||
Updated•25 years ago
|
Whiteboard: Have fix, will checkin as soon as tree as green.
Assignee | ||
Updated•25 years ago
|
Status: NEW → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 10•25 years ago
|
||
Fixed in Aug, 10, 199 3:42 build.
Updated•25 years ago
|
Status: RESOLVED → VERIFIED
Comment 11•25 years ago
|
||
Based on Kevin's comments, marking as verified fixed in the Aug 11th build.
You need to log in
before you can comment on or make changes to this bug.
Description
•