Bug 115248

I am going to try fixing this one.

After playing with css region width, min-width, max-width properties it seems that the region width is calculated properly for other cases as well (percent, calculated, viewport relative, ...). I will double check it and add a new test.

Seems that min-width: xx%; doesn't work when display: table; or display: table-cell; is set on the element, or parent element. See http://www.martialartworld.co.uk/ for an example - the top menu items *should* be equal widths (min-width: 14%), but the percentage is ignored. Tested in Chrome 27.0.1453.116 and Chrome 30.0.1559.0 canary. [note: http://www.martialartworld.co.uk/ is under development at the moment, but I've opened it up to viewing to demonstrate this issue, so the CSS is in a bit of a mess :-( ] Compare the menu viewed using Chrome to how it looks in Firefox 22.0 (latest release as of this date).

(In reply to comment #3) > Seems that > > min-width: xx%; > > doesn't work when > > display: table; > or > display: table-cell; > > is set on the element, or parent element. > > See http://www.martialartworld.co.uk/ for an example - the top menu items *should* be equal widths (min-width: 14%), but the percentage is ignored. > > Tested in Chrome 27.0.1453.116 and Chrome 30.0.1559.0 canary. > > [note: http://www.martialartworld.co.uk/ is under development at the moment, but I've opened it up to viewing to demonstrate this issue, so the CSS is in a bit of a mess :-( ] > > Compare the menu viewed using Chrome to how it looks in Firefox 22.0 (latest release as of this date). Thanks for the info. Would you be willing to create a small html sample that reproduces the problem and upload it here?

Created attachment 206384 [details] Screenshot of firefox example

Created attachment 206385 [details] Screenshot of chrome example

I have created a simple fiddle here: http://jsfiddle.net/chrisjg/q7nhQ/ example HTML <nav role="navigation"> <ul class="nav menu"> <li class="item-1">short</li> <li class="item-2">a bit longer</li> <li class="item-3">this is a lot longer</li> <li class="item-4">short</li> </ul> </nav> ----------------- example CSS nav { font-size: 16px; letter-spacing: -0.5px; list-style: none outside none; } .nav, nav ul { display: table; margin: 0; width: 100%; } .nav { float: left; left: 0; position: relative; list-style: none outside none; margin-bottom: 1.42857rem; margin-left: 0; width: 400px; } li { border: 1px solid black; display: table-cell; height: 34px; min-width: 25%; text-align: center; vertical-align: middle; } ------- The important bits are .nav display: table; li display: table-cell; li min-width: 25%; It should result in 4 equally sized boxes, as in the firefox screenshot I have attached. But in chrome it auto-sizes each box to fit the content - as in the chrome screenshot I also attached. Hope this helps. Chris.

Thanks! I will try convert to a ref test and fix the problem.

(In reply to comment #7) > I have created a simple fiddle here: http://jsfiddle.net/chrisjg/q7nhQ/ > > example HTML > > <nav role="navigation"> > <ul class="nav menu"> > <li class="item-1">short</li> > <li class="item-2">a bit longer</li> > <li class="item-3">this is a lot longer</li> > <li class="item-4">short</li> > </ul> > </nav> > ----------------- > > example CSS > > nav { > font-size: 16px; > letter-spacing: -0.5px; > list-style: none outside none; > } > .nav, nav ul { > display: table; > margin: 0; > width: 100%; > } > .nav { > float: left; > left: 0; > position: relative; > list-style: none outside none; > margin-bottom: 1.42857rem; > margin-left: 0; > width: 400px; > } > > li { > border: 1px solid black; > display: table-cell; > height: 34px; > min-width: 25%; > text-align: center; > vertical-align: middle; > } > > ------- > The important bits are > .nav display: table; > li display: table-cell; > li min-width: 25%; > > It should result in 4 equally sized boxes, > as in the firefox screenshot I have attached. But in chrome it auto-sizes each box to fit the content - as in the chrome screenshot I also attached. > > Hope this helps. > > Chris. Hi, Checked the example html now - I think it is probably intended for another bug or new bug should be created for this. From what I see it doesn't use CSS regions. Chris, could you confirm that - may be I am missing something.

Created attachment 208126 [details] Patch

Submitted new test to test min/max width cases for regions - the test is pass. There is an issue with red line in the last region which in my opinion is not related to this bug but should be investigate in a new one. If is ok I will create a new bug for the red line issue.

Comment on attachment 208126 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=208126&action=review > LayoutTests/ChangeLog:3 > + [CSSRegions] Min/max-width should support values other than length (This is an informal review) I think you should have a new pass on this test to clean it up a bit. Please note that I changed the bug description to make it more clear. > LayoutTests/fast/regions/region-min-max-width-support.html:1 > +<!doctype html> doctype -> DOCTYPE > LayoutTests/fast/regions/region-min-max-width-support.html:5 > +<style> You can reduce the style declarations in the (possible) following way: .box { height: 20px; border: 1px solid black; position: absolute; left: 200px; } .region { -webkit-flow-from: flow; } .reference { background-color: red; } #region1 { max-width: 1%; top: 100px;} #reference1 { max-width: 1%; top: 100px; } and later you can use the above classes when setting the properties for regions and their corresponding reference elements: <div id="reference1" class="box reference">region1</div> <div id="region1" class="box region"></div> > LayoutTests/fast/regions/region-min-max-width-support.html:6 > + #content { -webkit-flow-into: flow; background-color: green;} If you want to avoid seeing the red for the last region, you need to ensure that height of each slice of content flowed in each region has 20px. You can do that in the following way: body { font: 16px/1.25 monospace; } > LayoutTests/fast/regions/region-min-max-width-support.html:37 > +<body font-size: 20px; font-family: ahem;> Are these style declarations for body used? > LayoutTests/fast/regions/region-min-max-width-support.html:38 > +<p>Test for <a href="https://bugs.webkit.org/show_bug.cgi?id=115248"> [CSSRegions] Min/max-width should support values other than length</a>.</p> Please update the test with the new bug title. Also, it would be nice if you could add below something like: <p>On success you should see PASS below</p> > LayoutTests/fast/regions/region-min-max-width-support.html:78 > +document.body.offsetTop; // force layout Why do you need to force layout here? > LayoutTests/fast/regions/region-min-max-width-support.html:79 > +document.getElementById("region1").setAttribute("data-expected-width", document.getElementById('reference1').offsetWidth); All the below statements can be written in a for loop. > LayoutTests/fast/regions/region-min-max-width-support.html:91 > +checkLayout('#region1'); You can use something like this instead of repeating all the checkLayout statements: <body onload="checkLayout('.region')"> should you want to use a class for the region elements.

(In reply to comment #11) > Submitted new test to test min/max width cases for regions - the test is pass. There is an issue with red line in the last region which in my opinion is not related to this bug but should be investigate in a new one. If is ok I will create a new bug for the red line issue. The redness you are seeing in the last region is not a bug. You can avoid it by making sure that each slice of content has 20px, like i said in the review.

Comment on attachment 208126 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=208126&action=review >> LayoutTests/ChangeLog:3 >> + [CSSRegions] Min/max-width should support values other than length > > (This is an informal review) > I think you should have a new pass on this test to clean it up a bit. Please note that I changed the bug description to make it more clear. ok >> LayoutTests/fast/regions/region-min-max-width-support.html:1 >> +<!doctype html> > > doctype -> DOCTYPE ok >> LayoutTests/fast/regions/region-min-max-width-support.html:5 >> +<style> > > You can reduce the style declarations in the (possible) following way: > > .box { height: 20px; border: 1px solid black; position: absolute; left: 200px; } > .region { -webkit-flow-from: flow; } > .reference { background-color: red; } > > #region1 { max-width: 1%; top: 100px;} > #reference1 { max-width: 1%; top: 100px; } > > and later you can use the above classes when setting the properties for regions and their corresponding reference elements: > <div id="reference1" class="box reference">region1</div> > <div id="region1" class="box region"></div> ok >> LayoutTests/fast/regions/region-min-max-width-support.html:6 >> + #content { -webkit-flow-into: flow; background-color: green;} > > If you want to avoid seeing the red for the last region, you need to ensure that height of each slice of content flowed in each region has 20px. You can do that in the following way: > body { font: 16px/1.25 monospace; } yes, thanks for pointing it - made a mistake below as well. >> LayoutTests/fast/regions/region-min-max-width-support.html:37 >> +<body font-size: 20px; font-family: ahem;> > > Are these style declarations for body used? made a mistake here - forgot "style=""". >> LayoutTests/fast/regions/region-min-max-width-support.html:38 >> +<p>Test for <a href="https://bugs.webkit.org/show_bug.cgi?id=115248"> [CSSRegions] Min/max-width should support values other than length</a>.</p> > > Please update the test with the new bug title. Also, it would be nice if you could add below something like: > <p>On success you should see PASS below</p> ok >> LayoutTests/fast/regions/region-min-max-width-support.html:78 >> +document.body.offsetTop; // force layout > > Why do you need to force layout here? it was copy paste from other test - forgot to remove. >> LayoutTests/fast/regions/region-min-max-width-support.html:79 >> +document.getElementById("region1").setAttribute("data-expected-width", document.getElementById('reference1').offsetWidth); > > All the below statements can be written in a for loop. Yep, will look better >> LayoutTests/fast/regions/region-min-max-width-support.html:91 >> +checkLayout('#region1'); > > You can use something like this instead of repeating all the checkLayout statements: > <body onload="checkLayout('.region')"> should you want to use a class for the region elements. ok

Created attachment 208194 [details] Patch

(In reply to comment #15) > Created an attachment (id=208194) [details] > Patch LGTM

Comment on attachment 208194 [details] Patch r=me

Comment on attachment 208194 [details] Patch Clearing flags on attachment: 208194 Committed r154453: <http://trac.webkit.org/changeset/154453>

All reviewed patches have been landed. Closing bug.

(In reply to comment #16) > (In reply to comment #15) > > Created an attachment (id=208194) [details] > > Patch > > LGTM When i ran the final test case with webkit1 (r164000), im getting failed for Max-width cases. Min-width cases are passing. Needed help..Copying below the failed cases: FAIL: Expected 10 for width, but got 2. <div id="region1" class="box region" data-expected-width="10"></div> FAIL: Expected 10 for width, but got 2. <div id="region2" class="box region" data-expected-width="10"></div> FAIL: Expected 8 for width, but got 2. <div id="region3" class="box region" data-expected-width="8"></div> FAIL: Expected 18 for width, but got 2. <div id="region4" class="box region" data-expected-width="18"></div> FAIL: Expected 20 for width, but got 2. <div id="region5" class="box region" data-expected-width="20"></div> FAIL: Expected 3 for width, but got 2. <div id="region6" class="box region" data-expected-width="3"></div>