165590 2016-12-07 21:15:57 -0800 REGRESSION (r209490): [ios-simulator] LayoutTest fast/shadow-dom/media-shadow-manipulation.html is failing 2016-12-08 08:18:22 -0800 1 1 1 Unclassified WebKit Tools / Tests WebKit Nightly Build Unspecified Unspecified NEW P2 Normal --- 1 ryanhaddad webkit-unassigned ap bfulgham lforschler rniwa oldest_to_newest 1257594 0 ryanhaddad 2016-12-07 21:15:57 -0800 LayoutTest fast/shadow-dom/media-shadow-manipulation.html is failing https://build.webkit.org/results/Apple%20iOS%2010%20Simulator%20Release%20WK2%20(Tests)/r209515%20(2211)/results.html https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=fast%2Fshadow-dom%2Fmedia-shadow-manipulation.html --- /Volumes/Data/slave/ios-simulator-10-release-tests-wk2/build/layout-test-results/fast/shadow-dom/media-shadow-manipulation-expected.txt +++ /Volumes/Data/slave/ios-simulator-10-release-tests-wk2/build/layout-test-results/fast/shadow-dom/media-shadow-manipulation-actual.txt @@ -1,5 +1,6 @@ -PASS bounds = video_element.getBoundingClientRect(); document.caretRangeFromPoint(bounds.left + 10, bounds.bottom - 10) is null +FAIL bounds = video_element.getBoundingClientRect(); document.caretRangeFromPoint(bounds.left + 10, bounds.bottom - 10) should be null. Was . PASS successfullyParsed is true +Some tests failed. TEST COMPLETE 1257595 1 ryanhaddad 2016-12-07 21:16:09 -0800 Started with https://trac.webkit.org/changeset/209490 1257633 2 ap 2016-12-07 22:56:55 -0800 Just noticed iOS EWS falling behind others because of this test failing. 1257635 3 rniwa 2016-12-07 23:18:50 -0800 Ugh… we need to do more tweaking on this test :( Brent, what do you think about just deleting these tests? These tests aren’t doing anything useful after r209486. Alternatively we can just use internals.shadowRoot to get the shadow root directly. 1257657 4 rniwa 2016-12-08 00:05:06 -0800 It looks like this test wasn’t working even prior to my patch. The reason test is failing appears to stems from the video element being implemented differently on iOS and Mac. I really think the best approach here is to delete the test but I’d revert my change and the check conditional for now. 1257658 5 rniwa 2016-12-08 00:11:04 -0800 I fixed the test in https://trac.webkit.org/changeset/209529 for now but we should really just remove these tests. e.g. if we make this test use internals.shadowRoot, then it’s gonna hit the assertion in RenderImage.cpp we just added in https://trac.webkit.org/changeset/209431/trunk/Source/WebCore/rendering/RenderImage.cpp And this assertion is valid. Nobody should be inserting more than one mode as a direct child inside a media element’s shadow tree. So I don’t know how we’d make this test work without hitting any assertion. 1257700 6 bfulgham 2016-12-08 08:18:22 -0800 (In reply to comment #5) > I fixed the test in https://trac.webkit.org/changeset/209529 for now but we > should really just remove these tests. > > e.g. if we make this test use internals.shadowRoot, then it’s gonna hit the > assertion in RenderImage.cpp we just added in > https://trac.webkit.org/changeset/209431/trunk/Source/WebCore/rendering/ > RenderImage.cpp > > And this assertion is valid. Nobody should be inserting more than one mode > as a direct child inside a media element’s shadow tree. So I don’t know how > we’d make this test work without hitting any assertion. I'm fine with removing the test now that you've fixed the underlying issue.