RESOLVED FIXED 167538
[css-grid] LayoutTest fast/css-grid-layout/grid-simplified-layout-positioned.html is a flaky failure
https://bugs.webkit.org/show_bug.cgi?id=167538
Summary [css-grid] LayoutTest fast/css-grid-layout/grid-simplified-layout-positioned....
Attachments
Patch (3.28 KB, patch)
2017-02-10 03:41 PST, Manuel Rego Casasnovas
no flags
Patch for landing (3.42 KB, patch)
2017-02-13 02:17 PST, Manuel Rego Casasnovas
no flags
Archive of layout-test-results from ews101 for mac-elcapitan (930.15 KB, application/zip)
2017-02-13 03:18 PST, Build Bot
no flags
Archive of layout-test-results from ews106 for mac-elcapitan-wk2 (877.43 KB, application/zip)
2017-02-13 03:23 PST, Build Bot
no flags
Archive of layout-test-results from ews117 for mac-elcapitan (1.67 MB, application/zip)
2017-02-13 03:31 PST, Build Bot
no flags
New version with timeout (3.52 KB, patch)
2017-02-13 06:16 PST, Manuel Rego Casasnovas
no flags
Archive of layout-test-results from ews101 for mac-elcapitan (1.18 MB, application/zip)
2017-02-13 07:05 PST, Build Bot
no flags
Archive of layout-test-results from ews106 for mac-elcapitan-wk2 (911.46 KB, application/zip)
2017-02-13 07:21 PST, Build Bot
no flags
Archive of layout-test-results from ews116 for mac-elcapitan (1.67 MB, application/zip)
2017-02-13 07:27 PST, Build Bot
no flags
Patch for landing (3.77 KB, patch)
2017-04-11 00:57 PDT, Manuel Rego Casasnovas
no flags
Ryan Haddad
Comment 1 2017-01-27 17:32:43 PST
The images show that the test is expecting the cursor to be present in the text field, but it is missing.
Ryan Haddad
Comment 2 2017-02-09 15:20:35 PST
Ryan Haddad
Comment 3 2017-02-09 15:21:05 PST
This does not appear to be a recent regression. CC'ing test author.
Manuel Rego Casasnovas
Comment 4 2017-02-10 02:30:25 PST
It seems that the only issue is that the caret appears or not, the test is not checking anything regarding the caret. The only important thing was that with "autofocus" we were performing a simplified layout that was crashing before the patch that added the test (bug #163450). I'll modify the tests trying to get rid of this issue with the caret.
Manuel Rego Casasnovas
Comment 5 2017-02-10 03:41:43 PST
Created attachment 301146 [details] Patch Try to fix the test, let's see what EWS has to say.
Michael Catanzaro
Comment 6 2017-02-11 19:55:18 PST
Hm, I wonder if there is a more idiomatic way to force the cursor to not appear? Is the blur a pattern used by many tests to avoid this? When reading the test, I would be confused as to why the blur is present, since there's no comment. I'm kinda surprised that our test infrastructure doesn't have the ability to add two different expectations and allow the test to match either. Blinky cursors are a very common problem in automated tests and most test environments have a standard mechanism for dealing with them.
Darin Adler
Comment 7 2017-02-12 01:34:47 PST
Comment on attachment 301146 [details] Patch I am tempted to mark this as reviewed, but I am wondering: Is this still an effective test? Would the call to blur have side-stepped the crash even before we fixed the bug?
Darin Adler
Comment 8 2017-02-12 01:35:49 PST
Another possibility would be to enhance the test runners so that they understand the flashing caret and always take snapshots with the caret visible.
Darin Adler
Comment 9 2017-02-12 01:36:12 PST
Or always with the caret invisible, or any other sensible rule.
Darin Adler
Comment 10 2017-02-12 01:37:16 PST
Comment on attachment 301146 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=301146&action=review > LayoutTests/fast/css-grid-layout/grid-simplified-layout-positioned-expected.html:15 > <input autofocus> > +<script> > + document.querySelector("input").blur(); > +</script> Here in the expected file, maybe we can just omit autofocus instead of calling blur?
Michael Catanzaro
Comment 11 2017-02-12 05:00:01 PST
Comment on attachment 301146 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=301146&action=review > LayoutTests/ChangeLog:13 > + "autofocus" was needed to get the crash, so this change modifies > + the test to blur the element, so the caret is not painted anymore. autofocus was needed to get the crash.
Darin Adler
Comment 12 2017-02-12 10:49:13 PST
(In reply to comment #11) > autofocus was needed to get the crash. The expected file doesn’t need to crash. That’s why I suggested omitting it *in the expected file*, not in the test file.
Manuel Rego Casasnovas
Comment 13 2017-02-13 02:17:51 PST
Created attachment 301338 [details] Patch for landing
Manuel Rego Casasnovas
Comment 14 2017-02-13 02:19:42 PST
Thanks for the reviews. Uploaded new version for landing adding a comment to explain the "blur()" and also removing "autofocus" on the "-expected" file. (In reply to comment #7) > I am tempted to mark this as reviewed, but I am wondering: Is this still an > effective test? Would the call to blur have side-stepped the crash even > before we fixed the bug? Yeah I've verified that the test is still effective with the blur() part, as the crash was triggered before.
Build Bot
Comment 15 2017-02-13 03:18:36 PST
Comment on attachment 301338 [details] Patch for landing Attachment 301338 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/3077149 New failing tests: fast/css-grid-layout/grid-simplified-layout-positioned.html
Build Bot
Comment 16 2017-02-13 03:18:40 PST
Created attachment 301341 [details] Archive of layout-test-results from ews101 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews101 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Build Bot
Comment 17 2017-02-13 03:23:36 PST
Comment on attachment 301338 [details] Patch for landing Attachment 301338 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/3077154 New failing tests: fast/css-grid-layout/grid-simplified-layout-positioned.html
Build Bot
Comment 18 2017-02-13 03:23:39 PST
Created attachment 301342 [details] Archive of layout-test-results from ews106 for mac-elcapitan-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews106 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Build Bot
Comment 19 2017-02-13 03:31:29 PST
Comment on attachment 301338 [details] Patch for landing Attachment 301338 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/3077158 New failing tests: fast/css-grid-layout/grid-simplified-layout-positioned.html
Build Bot
Comment 20 2017-02-13 03:31:33 PST
Created attachment 301343 [details] Archive of layout-test-results from ews117 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews117 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Manuel Rego Casasnovas
Comment 21 2017-02-13 06:16:13 PST
Created attachment 301349 [details] New version with timeout
Manuel Rego Casasnovas
Comment 22 2017-02-13 06:18:52 PST
Oops, so in my previous version the "blur()" was run before "autofocus", so it was not useful at all. Both versions the test and the expected file were showing the caret (so probably it would be still flacky on Mac. New version uses setTimeout() on the test file to ensure that blur() is run after "autofocus". Would you mind to take a new look? Thanks.
Build Bot
Comment 23 2017-02-13 07:05:19 PST
Comment on attachment 301349 [details] New version with timeout Attachment 301349 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/3077880 New failing tests: fast/css-grid-layout/grid-simplified-layout-positioned.html
Build Bot
Comment 24 2017-02-13 07:05:23 PST
Created attachment 301350 [details] Archive of layout-test-results from ews101 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews101 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Build Bot
Comment 25 2017-02-13 07:21:29 PST
Comment on attachment 301349 [details] New version with timeout Attachment 301349 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/3077922 New failing tests: fast/css-grid-layout/grid-simplified-layout-positioned.html
Build Bot
Comment 26 2017-02-13 07:21:34 PST
Created attachment 301354 [details] Archive of layout-test-results from ews106 for mac-elcapitan-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews106 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Build Bot
Comment 27 2017-02-13 07:27:28 PST
Comment on attachment 301349 [details] New version with timeout Attachment 301349 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/3077921 New failing tests: fast/css-grid-layout/grid-simplified-layout-positioned.html
Build Bot
Comment 28 2017-02-13 07:27:32 PST
Created attachment 301355 [details] Archive of layout-test-results from ews116 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews116 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Manuel Rego Casasnovas
Comment 29 2017-02-14 08:05:38 PST
Comment on attachment 301349 [details] New version with timeout Oops so this is still not working on Mac. I'd need to take a deeper look when I find some time, removing from review queue at this point.
Manuel Rego Casasnovas
Comment 30 2017-04-11 00:57:08 PDT
Created attachment 306784 [details] Patch for landing Fix test so it waits until the blur() happens, this makets it passes on Mac.
WebKit Commit Bot
Comment 31 2017-04-11 02:20:20 PDT
Comment on attachment 306784 [details] Patch for landing Clearing flags on attachment: 306784 Committed r215222: <http://trac.webkit.org/changeset/215222>
WebKit Commit Bot
Comment 32 2017-04-11 02:20:22 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.