REGRESSION (r167818): editing/inserting/typing-space-to-trigger-smart-link.html fails on WebKit1 bots

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Tim Horton]

http://build.webkit.org/results/Apple%20Mavericks%20Release%20WK1%20(Tests)/r167820%20(5521)/results.html
http://webkit-test-results.appspot.com/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=editing%2Finserting%2Ftyping-space-to-trigger-smart-link.html

[Radar WebKit Bug Importer]

<rdar://problem/16730393>

[Tim Horton]

-PASS: the anchor for 'www.foo.com' has been created.
+Failed: the expected content was 'The <a href="http://www.foo.com">www.foo.com</a> should be underlined and there is an anchor node created for it.', but the actual result was 'The www.foo.com should be underlined and there is an anchor node created for it.'.

[Tim Horton]

Marked in http://trac.webkit.org/changeset/167827 for WK1

[David Kilzer (:ddkilzer)]

Assuming the test is failing because we're hitting the reentrancy guard added by r167818, we should probably break runTest() in the layout test into multiple pieces (using setTimeout() calls).

Yep, that fixes the test.

[David Kilzer (:ddkilzer)]

(In reply to comment #4)
> Assuming the test is failing because we're hitting the reentrancy guard added by r167818, we should probably break runTest() in the layout test into multiple pieces (using setTimeout() calls).
> 
> Yep, that fixes the test.

Correction, I must not have been running against the build of WebKit I thought I was.

[David Kilzer (:ddkilzer)]

Created attachment 230261 [details]
Incorrect fix: use setTimeout() in test

Perhaps the re-entrant behavior needs to be fixed in WebCore itself.

[Jon Honeycutt]

Created attachment 231199 [details]
Patch

[Darin Adler]

Comment on attachment 231199 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=231199&action=review

> Source/WebCore/editing/ApplyStyleCommand.cpp:816
>      for (size_t i = 0; i < runs.size(); i++) {
> -        removeConflictingInlineStyleFromRun(style, runs[i].start, runs[i].end, runs[i].pastEndNode);
> -        runs[i].positionForStyleComputation = positionToComputeInlineStyleChange(runs[i].start, runs[i].dummyElement);
> +        InlineRunToApplyStyle& run = runs[i];

The better way to write this is:

    for (auto& run : runs) {

[Jon Honeycutt]

Committed r168641: <http://trac.webkit.org/changeset/168641>