Truitt Savell 2019-04-30 09:56:55 PDT It looks like the changes in https://trac.webkit.org/changeset/244494/webkit has caused the test editing/pasteboard/5761530-1.html to become flakey. I was able to reproduce this using command: run-webkit-tests editing/pasteboard/5761530-1.html --iterations 20 --ios-simulator The test passes on 244493 and with fail on 244494 History: https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=editing%2Fpasteboard%2F5761530-1.html Diff: --- /Volumes/Data/slave/ios-simulator-12-release-tests-wk2/build/layout-test-results/editing/pasteboard/5761530-1-expected.txt +++ /Volumes/Data/slave/ios-simulator-12-release-tests-wk2/build/layout-test-results/editing/pasteboard/5761530-1-actual.txt @@ -1,3 +1,3 @@ This tests to see that tabs are put into tab spans when they are copied individually. The pasted tab should be inside of a tab span, not a style span. To run the test manually, paste and then inspect the editable region, and ensure that there is a tab span at the beginning of the editable div. -<span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-tab-span" style="white-space:pre;"> </span>xxx +<span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-tab-span" style="white-space:pre;"> </span>xxx
<rdar://problem/50348091>
It was specifically this change: --- a/Source/WebKit/WebProcess/WebPage/WebPage.cpp +++ b/Source/WebKit/WebProcess/WebPage/WebPage.cpp @@ -1125,7 +1125,7 @@ void WebPage::updateEditorStateAfterLayoutIfEditabilityChanged() Frame& frame = m_page->focusController().focusedOrMainFrame(); EditorStateIsContentEditable editorStateIsContentEditable = frame.selection().selection().isContentEditable() ? EditorStateIsContentEditable::Yes : EditorStateIsContentEditable::No; if (m_lastEditorStateWasContentEditable != editorStateIsContentEditable) - sendPartialEditorStateAndSchedulePostLayoutUpdate(); + scheduleFullEditorStateUpdate(); } String WebPage::renderTreeExternalRepresentation() const ...that causes this test to be flaky. Reverting to always send the partial editor state update immediately when changing editability fixes the flakiness for me. I'll need to see what exactly in the UI process is depending on this state being up to date.
...this is even weirder than I thought! Simply asking whether the current selection is at the beginning of a sentence via FrameSelection::selectionAtSentenceStart (and not even using the result) in this function: Frame& frame = m_page->focusController().focusedOrMainFrame(); EditorStateIsContentEditable editorStateIsContentEditable = frame.selection().selection().isContentEditable() ? EditorStateIsContentEditable::Yes : EditorStateIsContentEditable::No; - if (m_lastEditorStateWasContentEditable != editorStateIsContentEditable) + if (m_lastEditorStateWasContentEditable != editorStateIsContentEditable) { + frame.selection().selectionAtSentenceStart(); scheduleFullEditorStateUpdate(); + } ...seems to make this test pass reliably 🤔.
Oh, I wonder this is an issue with simple line layout. Try disabling it via settings?
(In reply to Ryosuke Niwa from comment #4) > Oh, I wonder this is an issue with simple line layout. Try disabling it via > settings? Oh interesting! Adding `internals.settings.setSimpleLineLayoutEnabled(false);` to the beginning of the test does seem to fix the flakiness...
Okay. So I think this might be a bug in TextIterator / StylizedMarkupAccumulator with simple line layout code in which a tab character or 4 spaces is collapsed in its serialization even though white-space: pre is set.
It's probably a bug in TextIterator::handleTextNode. When copying the content of a plain text, we end up using TextIterator to grab the content in StyledMarkupAccumulator::appendText
(In reply to Ryosuke Niwa from comment #7) > It's probably a bug in TextIterator::handleTextNode. When copying the > content of a plain text, we end up using TextIterator to grab the content in > StyledMarkupAccumulator::appendText Yep, I'm poking around in and around this code at the moment :) I'll post some more findings in a bit. There's some behaviors regarding the lifetime of a renderer's SLL that are currently confusing me.
Created attachment 368708 [details] Reduction Here's a reduction of the issue Zalan and I worked.
Alan, Ryosuke and I chatted about this over IRC. For now, let's disable SLL in this test to make it pass; Alan will continue investigating a fix in SLL logic to get innerText working for tab-spans in SLL.
Created attachment 368714 [details] Patch
Comment on attachment 368714 [details] Patch Clearing flags on attachment: 368714 Committed r244854: <https://trac.webkit.org/changeset/244854>
All reviewed patches have been landed. Closing bug.
Reopening to track the SLL issue described above.