third_party/WebKit/LayoutTests/fast/table/table-size-integer-overflow.html - Issue 1921973005: [css tables] Don't pass table width increase due to % columns to parents

Side by Side Diff: third_party/WebKit/LayoutTests/fast/table/table-size-integer-overflow.html

Issue 1921973005: [css tables] Don't pass table width increase due to % columns to parents (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master

Patch Set: move variables around Created 4 years, 7 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View unified diff | Download patch

« third_party/WebKit/LayoutTests/fast/table/max-width-integer-overflow.html ('K') | « third_party/WebKit/LayoutTests/fast/table/max-width-integer-overflow-expected.txt ('k') | third_party/WebKit/LayoutTests/fast/table/table-size-integer-overflow-expected.txt » ('j') | third_party/WebKit/Source/core/layout/LayoutTable.cpp » ('J')
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

OLD	NEW
1 <html>

2 <head>

3 <script>

4 window.onload = function() {

5 if (window.testRunner)

6 window.testRunner.dumpAsText();

7

8 var cell = document.getElementById("cell");

9 var text = cell.firstElementChild;

10 var wdiff = cell.offsetWidth - text.offsetWidth - (parseInt(window.getComputed Style(cell).getPropertyValue('padding-right')) +

11 parseInt(window.getComputed Style(cell).getPropertyValue('padding-left')));

12 if (wdiff > 0)

13 text.innerText = "PASS";

14 }

15 </script>

16 </head>

17 <body>

18 <div style="float: left;">	1 <div style="float: left;">
	dgrogan 2016/04/27 03:14:04 The float makes a difference here but I am not yet The float makes a difference here but I am not yet sure why. mstensho (USE GERRIT) 2016/04/27 12:19:48 An auto-width float is shrink-to-fit, which might Show quoted text On 2016/04/27 03:14:04, dgrogan wrote: > The float makes a difference here but I am not yet sure why. An auto-width float is shrink-to-fit, which might affect percentage resolution inside the table, maybe? But it seems wrong that your change affects the behavior here. I think it should be limited to nested tables, and not affect how we lay out a table inside a shrink-to-fit container. dgrogan 2016/04/27 20:24:05 Yeah, if that matches blink's current behavior and Show quoted text On 2016/04/27 12:19:48, mstensho wrote: > On 2016/04/27 03:14:04, dgrogan wrote: > > The float makes a difference here but I am not yet sure why. > > An auto-width float is shrink-to-fit, which might affect percentage resolution > inside the table, maybe? But it seems wrong that your change affects the > behavior here. I think it should be limited to nested tables, and not affect how > we lay out a table inside a shrink-to-fit container. Yeah, if that matches blink's current behavior and Edge, then I agree we should stick to it.
19 <table style="margin: 1px">	2 <table style="margin: 1px" data-expected-width="40">

20 <tr>	3 <tr>

21 <td></td>	4 <td></td>

22 <td id="cell" style="background-color: green; width: 100%; height: 30px">	5 <td id="cell" style="background-color: green; width: 100%; height: 30px" d ata-expected-width="32">

23 <span>FAIL</span>	6 <div style="display:inline-block; width:30px;"></div>

24 </td>	7 </td>

25 </tr>	8 </tr>

26 </table>	9 </table>

27 </div>	10 </div>

28 <div style="clear: left;">The green box should be the full width of the page.</d iv>	11 <div style="clear: left;">Edge 25.10586.0.0 shows the green box as the full widt h of the page. So did Blink before Chrome 52. FireFox 48 agrees with Chrome 52.< /div>
	dgrogan 2016/04/27 03:14:04 I am a little worried that this is a bellwether fo I am a little worried that this is a bellwether for breaking mobile webview content, especially if it makes us diverge from webkit, which I haven't tested. mstensho (USE GERRIT) 2016/04/27 12:19:48 Diverging from Edge for table layout certainly doe Show quoted text On 2016/04/27 03:14:04, dgrogan wrote: > I am a little worried that this is a bellwether for breaking mobile webview > content, especially if it makes us diverge from webkit, which I haven't tested. Diverging from Edge for table layout certainly doesn't sound too good, no. I wonder if this one fails now, too: <!DOCTYPE html> <p>There should be a blue square below.</p> <div style="float:left;"> <table cellspacing="0" cellpadding="0" style="background:blue;"> <td style="width:20%;"> <div style="width:40px; height:200px;"></div> </td> <td></td> </table> </div> I have problems applying your patch, due to changes in binary files, so I haven't tested. dgrogan 2016/04/27 20:24:05 Yeah, this one we also change behavior from Edge t Show quoted text On 2016/04/27 12:19:48, mstensho wrote: > On 2016/04/27 03:14:04, dgrogan wrote: > > I am a little worried that this is a bellwether for breaking mobile webview > > content, especially if it makes us diverge from webkit, which I haven't > tested. > > Diverging from Edge for table layout certainly doesn't sound too good, no. > > I wonder if this one fails now, too: > > <!DOCTYPE html> > <p>There should be a blue square below.</p> > <div style="float:left;"> > <table cellspacing="0" cellpadding="0" style="background:blue;"> > <td style="width:20%;"> > <div style="width:40px; height:200px;"></div> > </td> > <td></td> > </table> > </div> > > I have problems applying your patch, due to changes in binary files, so I > haven't tested. Yeah, this one we also change behavior from Edge to FF.
29 </body>	12 <script src="../../resources/check-layout.js"></script>

30 </html>	13 <script>

	14 checkLayout('table');

	15 </script>

OLD	NEW