Bug 16148

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:

Keywords:

HasReduction

Version:

528+ (Nightly build)

Hardware:

Mac (Intel)

OS:

OS X 10.5

Attachments:

Description	Flags
testcase which demonstrates the rendering issue	none
Patch calcPrefWidths (use specified width as min/max pref width when 0).	hyatt: review-

Dieter Komendera

Reported 2007-11-26 14:14:27 PST

There is an issue when a parent div (float:left, overflow:hidden) contains several child divs (also float:left, overflow:hidden). If one or more of the child divs have a width set to 0, and have content (eg also a div with a specific width), the parent div will then be rendered as of the child divs hadn't set the width to 0. I'll attach an example HTML file which demonstrates the problem. This works for me as expected in Firefox 2.0.

Attachments
testcase which demonstrates the rendering issue (1.33 KB, text/html) 2007-11-26 14:15 PST, Dieter Komendera	no flags	Details
Patch calcPrefWidths (use specified width as min/max pref width when 0). (5.12 KB, patch) 2009-05-28 12:12 PDT, Jacob Refstrup	hyatt: review-	Details Formatted Diff Diff
View All Add attachment proposed patch, testcase, etc.

Dieter Komendera

Comment 1 2007-11-26 14:15:34 PST

Created attachment 17537 [details] testcase which demonstrates the rendering issue

Jacob Refstrup

Comment 2 2009-05-28 12:12:54 PDT

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.

Eric Seidel (no email)

Comment 3 2009-06-06 01:14:45 PDT

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.

Dave Hyatt

Comment 4 2009-06-06 13:00:53 PDT

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.

Jacob Refstrup

Comment 5 2009-06-08 23:06:25 PDT

(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())); } ?

Robert Hogan

Comment 6 2012-12-26 07:22:27 PST

The test case no longer fails as of r138211. OK to close?

Dieter Komendera

Comment 7 2012-12-26 14:24:08 PST

nice, thanks! ok to close from my side.

Note You need to log in before you can comment on or make changes to this bug.