Bug 165590 – REGRESSION (r209490): [ios-simulator] LayoutTest fast/shadow-dom/media-shadow-manipulation.html is failing

• Component: Tools / Tests
• Status: NEW

[Ryan Haddad]

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

[Ryan Haddad]

Started with https://trac.webkit.org/changeset/209490

[Alexey Proskuryakov]

Just noticed iOS EWS falling behind others because of this test failing.

[Ryosuke Niwa]

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.

[Ryosuke Niwa]

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.

[Ryosuke Niwa]

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.

[Brent Fulgham]

(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.