Summary: | overflow: hidden with float:left rendering issue | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Dieter Komendera <dieter> | ||||||
Component: | Layout and Rendering | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | emacemac7, hyatt, jacob.refstrup, mitz, robert | ||||||
Priority: | P2 | Keywords: | HasReduction | ||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Mac (Intel) | ||||||||
OS: | OS X 10.5 | ||||||||
Attachments: |
|
Description
Dieter Komendera
2007-11-26 14:14:27 PST
Created attachment 17537 [details]
testcase which demonstrates the rendering issue
Created attachment 30742 [details]
Patch calcPrefWidths (use specified width as min/max pref width when 0).
It seems that comparing width > 0 is fairly common; there's also table cell code that does this. Perhaps a similar fix should at least be made for the table cell case -- not sure if it'd make a difference for min-width or max-width in calcPrefWidths.
Comment on attachment 30742 [details]
Patch calcPrefWidths (use specified width as min/max pref width when 0).
Your test case could be made much smaller by removing unneeded lines of HTML. (e.g. the DOCTYPE, the entire <head> a little <style> tag would also kill some of the copy/paste style=). It's also missing a newline at the end of the file.
Hyatt should review this.
Comment on attachment 30742 [details]
Patch calcPrefWidths (use specified width as min/max pref width when 0).
Pretty sure it's impossible for width to be negative, so once you've changed it to >= 0 it's equivalent to just not having that in the if statement at all.
Also, you need to patch RenderFlexibleBox::calcPrefWidths, as it has the same mistake.
(In reply to comment #4) > (From update of attachment 30742 [details] [review]) > Pretty sure it's impossible for width to be negative, so once you've changed it > to >= 0 it's equivalent to just not having that in the if statement at all. > > Also, you need to patch RenderFlexibleBox::calcPrefWidths, as it has the same > mistake. > Good catch :-) I'll go ahead and remove the >= 0 and fix the RenderFlexibleBox::calcPrefWidths and simplify the added test case. Your opinion on whether a similar fix should be applied to the tableCell case that's approx 21 lines below: if (isTableCell()) { Length w = static_cast<const RenderTableCell*>(this)->styleOrColWidth(); if (w.isFixed() && w.value() > 0) m_maxPrefWidth = max(m_minPrefWidth, calcContentBoxWidth(w.value())); } ? The test case no longer fails as of r138211. OK to close? nice, thanks! ok to close from my side. |