Bug 161366 – REGRESSION (r205094): LayoutTest editing/pasteboard/5478250.html is a flaky failure

• Status: RESOLVED
• Resolution: FIXED
• Component: HTML Editing

[Ryan Haddad]

REGRESSION: LayoutTest editing/pasteboard/5478250.html is a flaky failure

https://build.webkit.org/builders/Apple%20El%20Capitan%20Debug%20WK2%20(Tests)/builds/7837

https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=editing%2Fpasteboard%2F5478250.html

--- /Volumes/Data/slave/elcapitan-debug-tests-wk2/build/layout-test-results/editing/pasteboard/5478250-expected.txt
+++ /Volumes/Data/slave/elcapitan-debug-tests-wk2/build/layout-test-results/editing/pasteboard/5478250-actual.txt
@@ -17,7 +17,8 @@
             RenderText {#text} at (111,0) size 35x18
               text run at (111,0) width 35: "bold."
         RenderBlock {DIV} at (0,18) size 784x18
-          RenderInline {SPAN} at (0,0) size 398x18
-            RenderText {#text} at (0,0) size 398x18
-              text run at (0,0) width 398: "This text should bold and left justified with \"Some text...\"."
-caret: position 61 of child 0 {#text} of child 0 {SPAN} of child 2 {DIV} of child 4 {DIV} of body
+          RenderInline {SPAN} at (0,0) size 402x18
+            RenderInline {B} at (0,0) size 402x18
+              RenderText {#text} at (0,0) size 402x18
+                text run at (0,0) width 402: "This text should bold and left justified with \"Some text\x{2026}\"."
+caret: position 59 of child 0 {#text} of child 0 {B} of child 0 {SPAN} of child 2 {DIV} of child 4 {DIV} of body

[Ryan Haddad]

Earliest failure seen with:
https://trac.webkit.org/log/?verbose=on&rev=205094&stop_rev=205093

[Ryan Haddad]

Maybe related to https://trac.webkit.org/changeset/205094

[Ryan Haddad]

Marked test as flaky on mac-wk2 debug in http://trac.webkit.org/projects/webkit/changeset/205187

[Jonathan Bedard]

Just a few comments so that this is documented.

I find it highly unlikely that it was https://trac.webkit.org/changeset/205094.  This change added a function to the test runner that is only called by two tests, which were failing before the change.  editing/pasteboard/5478250.html does not call the added function.  Looking at the immediate preceding change, however, https://trac.webkit.org/changeset/205093, may be a potential culprit, as the error we see in editing.pasteboard/5478250.html has to do with CSS.

I suspect that this bug depends on test order, although I have not managed to reproduce it, so I cannot definitively say that is what is going on.

[Jonathan Bedard]

I'm actually curious as to why this test is passing at all.  Opening it in Safari seems to indicate the test should be failing 100% of the time.  Oddly, I can't get it to fail in the test suite, it seems like it should be failing there too if it is failing in Safari.

Test results:
https://build.webkit.org/results/Apple%20El%20Capitan%20Debug%20WK2%20%28Tests%29/r205108%20%287815%29/results.html

[Alexey Proskuryakov]

I can reproduce locally. The difference almost certainly means that text replacement now fires, and changes three dots into an ellipsis, so r205094 does seem somewhat related.

> I'm actually curious as to why this test is passing at all.

The test only verifies that the text is bold when running as a pixel test - render tree doesn't encode whether the text was bold. So that regression occurred long ago, unnoticed. We'll need a separate bug for that.

[Alexey Proskuryakov]

Steps to reproduce: 

run-webkit-tests --child-processes=1 editing/inserting/smart-link-when-caret-is-moved-before-URL.html editing/pasteboard/5478250.html  -v --no-retry

[Radar WebKit Bug Importer]

<rdar://problem/28171977>

[Alexey Proskuryakov]

Bug 161411 seems like it probably shares the root cause with this.

[Jonathan Bedard]

*** Bug 161411 has been marked as a duplicate of this bug. ***

[Jonathan Bedard]

This is because of a failure to reset state when a new test begins, specifically due to setAutomaticLinkDetectionEnabled(bool) not reseting.

[Ryan Haddad]

Removed flaky expectations and skipped editing/inserting/smart-link-when-caret-is-moved-before-URL.html in https://trac.webkit.org/r205491

[Jonathan Bedard]

Further investigation has revealed that the "setAutomaticLinkDetectionEnabled" flag is reset before each test (although, many of the TestRunner's functions are not reset before each test).  The flag also remains reset through out the test.  Yet, somehow state is being retained between tests.

[Jonathan Bedard]

This is a bug with the TestRunner.  The test runner is attempting to modify the text checker from the WebContent process instead of the UI process.

[Jonathan Bedard]

To fully fix this bug, API additions will need to be made.

It is unclear why WKTextChecker.h, the file which provides these checks for most platforms, is excluded on Apple's builds.  The primary functionality provided, spelling and grammar checking, are defined by Apple's text checker.

[Jonathan Bedard]

https://bugs.webkit.org/show_bug.cgi?id=161919 contains the more permanent fix to this issue.

[Jonathan Bedard]

This issue has been fixed as of r206261: <http://trac.webkit.org/changeset/206261>.