RESOLVED FIXED 130770
[CSS Grid Layout] Prevent issues with checkLayout() in grid items 
https://bugs.webkit.org/show_bug.cgi?id=130770
Summary [CSS Grid Layout] Prevent issues with checkLayout() in grid items
Manuel Rego Casasnovas
Reported 2014-03-26 01:27:33 PDT
When we use checkLayout() on a grid item, the results (PASS or FAIL messages) are added as new auto-placed items in the grid. This might cause some unexpected behavior in the tests. In the current code all of them are passing anyway, but depending on the specific test some problems could arise. It seems a better idea to modify the related layout tests to use a different container for the test results. Avoiding any kind of interference with the grid code. This has been already fixed in Blink: https://codereview.chromium.org/208133003/
Attachments
Patch (22.45 KB, patch)
2014-03-26 01:34 PDT, Manuel Rego Casasnovas 		no flags
DetailsFormatted DiffDiff
Patch (4.58 KB, patch)
2014-03-26 03:12 PDT, Manuel Rego Casasnovas 		no flags
DetailsFormatted DiffDiff
Patch (4.62 KB, patch)
2014-03-26 03:40 PDT, Manuel Rego Casasnovas 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Manuel Rego Casasnovas
Comment 1 2014-03-26 01:34:37 PDT
Created attachment 227835 [details] Patch
Sergio Villar Senin
Comment 2 2014-03-26 01:59:56 PDT
Comment on attachment 227835 [details] Patch There is an issue in the tests indeed, but moving the output to a single place worsens the expectations as it becomes more difficult to know which tests could be failing. Could it be possible to fix the issue without changing the expectations?
Manuel Rego Casasnovas
Comment 3 2014-03-26 03:12:12 PDT
Created attachment 227842 [details] Patch I modified the patch to keep the same expectations. On top of that I realized that the the patch contained some uneeded changes, becaues of checkLayout for a grid is not a problem as results will be added to the parent and not the grid itself.
Sergio Villar Senin
Comment 4 2014-03-26 03:27:51 PDT
Comment on attachment 227842 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=227842&action=review Much better indeed. > LayoutTests/ChangeLog:18 > + Nit: could you please try to keep columns <80 before landing? Readability of the ChangeLog is improved a lot.
Manuel Rego Casasnovas
Comment 5 2014-03-26 03:40:18 PDT
Created attachment 227843 [details] Patch Patch for landing.
WebKit Commit Bot
Comment 6 2014-03-26 04:12:20 PDT
Comment on attachment 227843 [details] Patch Clearing flags on attachment: 227843 Committed r166290: <http://trac.webkit.org/changeset/166290>
WebKit Commit Bot
Comment 7 2014-03-26 04:12:23 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.