Closed
Bug 381074
Opened 17 years ago
Closed 17 years ago
Hang with display: table-caption, large padding and left float inside it
Categories
(Core :: Layout: Floats, defect)
Tracking
()
VERIFIED
FIXED
People
(Reporter: martijn.martijn, Assigned: dholbert)
References
Details
(Keywords: hang, regression, testcase)
Attachments
(2 files, 1 obsolete file)
(deleted),
text/html
|
Details | |
(deleted),
patch
|
bzbarsky
:
review+
roc
:
superreview+
|
Details | Diff | Splinter Review |
See testcase, which hangs current trunk Mozilla builds on load. It doesn't hang in branch builds. If wanted, I could find out a regression range for this.
Reporter | ||
Comment 1•17 years ago
|
||
Ok, so this regressed between 2007-02-06 and 2007-02-08, which is when the patch for bug 177805 went in, so I guess this is somehow a regression from bug 177805.
Blocks: pixels
Keywords: regression
Comment 2•17 years ago
|
||
It's hanging in nsSpaceManager::GetBandAvailableSpace in the first while loop; somehow there's a cycle in the band list.
Reporter | ||
Updated•17 years ago
|
Component: Layout: Tables → Layout: Floats
QA Contact: layout.tables → layout.floats
Comment 3•17 years ago
|
||
It's probably getting confused by the negative padding...
Comment 4•17 years ago
|
||
Negative padding shouldn't get beyond the CSS parser -- it's a syntax error.
Comment 5•17 years ago
|
||
(In reply to comment #4) > Negative padding shouldn't get beyond the CSS parser -- it's a syntax error. The issue isn't the parsed value; it's the computed value. According to the DOM inspector, the style rule says padding-top: 1e+8px, but the computed value is padding-top: -1.78957e+7px.
Comment 7•17 years ago
|
||
(In reply to comment #6) > Did we overflow somewhere or something? I think so.
Updated•17 years ago
|
Flags: blocking1.9? → blocking1.9+
Comment 8•17 years ago
|
||
Er, of course. We have 1e8 pixels, 60 app units per pixel, so 6e9 app units. That doesn't fit in 32 bits, of course, much less the 30 bits nscoord actually has. Not sure what we want to do here, short of going to a float nscoord...
Assignee | ||
Comment 9•17 years ago
|
||
The conversion that causes the nscoord overflow is happening here: nsRuleNode.cpp:297 (in SetCoord) nsRuleNode.cpp:215 (in CalcLength) nsPresContext.cpp:490 (in CSSPixelsToAppUnits) nsUnitConversion.h:91 (in NSFloatPixelsToAppUnits) with the faulty line being: 100 nscoord r = aPixels * (nscoord)aAppUnitsPerPixel; My patch just bounds-checks the multiplication product before converting it into a nscoord.
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
Assignee | ||
Comment 10•17 years ago
|
||
Added an #ifdef NS_COORD_IS_FLOAT case, so that we won't bother with the new bounds-checking after nscoords become floats.
Attachment #280006 -
Attachment is obsolete: true
Assignee | ||
Updated•17 years ago
|
Attachment #280009 -
Flags: review?(bzbarsky)
Assignee | ||
Updated•17 years ago
|
OS: Windows XP → All
Comment 11•17 years ago
|
||
Comment on attachment 280009 [details] [diff] [review] patch (adding NS_COORD_IS_FLOAT case) r=bzbarsky, but watch for performance impact!
Attachment #280009 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Updated•17 years ago
|
Attachment #280009 -
Flags: superreview?(roc)
Attachment #280009 -
Flags: superreview?(roc) → superreview+
Assignee | ||
Comment 12•17 years ago
|
||
Patch checked in to trunk. --> Fixed. Checking in nsUnitConversion.h; /cvsroot/mozilla/xpcom/ds/nsUnitConversion.h,v <-- nsUnitConversion.h new revision: 3.17; previous revision: 3.16 done
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 13•17 years ago
|
||
Verified fixed, using: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a9pre) Gecko/2007092605 Minefield/3.0a9pre
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•