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
Started with https://trac.webkit.org/changeset/209490
Just noticed iOS EWS falling behind others because of this test failing.
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.
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.
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.
(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.