Bug 132207

Summary: REGRESSION (r167818): editing/inserting/typing-space-to-trigger-smart-link.html fails on WebKit1 bots
Product: WebKit Reporter: Tim Horton <thorton>
Component: Tools / TestsAssignee: Jon Honeycutt <jhoneycutt>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, ddkilzer, enrica, jhoneycutt, msaboff, rniwa, thorton, webkit-bug-importer
Priority: P2 Keywords: InRadar, LayoutTestFailure, Regression
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Incorrect fix: use setTimeout() in test
none
Patch darin: review+

Comment 1 Radar WebKit Bug Importer 2014-04-25 14:43:44 PDT
<rdar://problem/16730393>
Comment 2 Tim Horton 2014-04-25 14:44:05 PDT
-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.'.
Comment 3 Tim Horton 2014-04-25 15:03:32 PDT
Marked in http://trac.webkit.org/changeset/167827 for WK1
Comment 4 David Kilzer (:ddkilzer) 2014-04-27 09:18:03 PDT
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.
Comment 5 David Kilzer (:ddkilzer) 2014-04-27 10:20:06 PDT
(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.
Comment 6 David Kilzer (:ddkilzer) 2014-04-27 10:25:59 PDT
Created attachment 230261 [details]
Incorrect fix: use setTimeout() in test

Perhaps the re-entrant behavior needs to be fixed in WebCore itself.
Comment 7 Jon Honeycutt 2014-05-09 17:51:12 PDT
Created attachment 231199 [details]
Patch
Comment 8 Darin Adler 2014-05-10 11:01:09 PDT
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) {
Comment 9 Jon Honeycutt 2014-05-12 13:32:49 PDT
Committed r168641: <http://trac.webkit.org/changeset/168641>