Bug 187253 - [WK1] editing/spelling/markers.html is failing on recent builds of macOS Mojave
Summary: [WK1] editing/spelling/markers.html is failing on recent builds of macOS Mojave
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Wenson Hsieh
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-07-02 09:42 PDT by Wenson Hsieh
Modified: 2018-07-05 08:47 PDT (History)
7 users (show)

See Also:


Attachments
Patch (29.67 KB, patch)
2018-07-02 13:17 PDT, Wenson Hsieh
thorton: review+
Details | Formatted Diff | Diff
Patch for landing (29.74 KB, patch)
2018-07-02 14:14 PDT, Wenson Hsieh
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Wenson Hsieh 2018-07-02 09:42:15 PDT
This started in recent builds of macOS Mojave. What appears to be happening is that the system spell checker is now returning both grammar and spelling results for the "I have a issue." portion of the test, and our test isn't robust enough to handle this.
Comment 1 Wenson Hsieh 2018-07-02 13:17:53 PDT
Created attachment 344129 [details]
Patch
Comment 2 Tim Horton 2018-07-02 13:28:51 PDT
Comment on attachment 344129 [details]
Patch

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

> Tools/TestRunnerShared/cocoa/LayoutTestSpellChecker.mm:228
> +            auto resultsObject = JSValueToObject(context, JSObjectGetPropertyAtIndex(context, resultsArray, resultIndex, nullptr), nullptr);

Someday we should make this use the modern ObjC JSC API.
Comment 3 Wenson Hsieh 2018-07-02 13:42:23 PDT
Comment on attachment 344129 [details]
Patch

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

>> Tools/TestRunnerShared/cocoa/LayoutTestSpellChecker.mm:228
>> +            auto resultsObject = JSValueToObject(context, JSObjectGetPropertyAtIndex(context, resultsArray, resultIndex, nullptr), nullptr);
> 
> Someday we should make this use the modern ObjC JSC API.

Sounds good! I'll refactor this the next time I touch add new functionality here, and add a FIXME to remind myself.
Comment 4 Wenson Hsieh 2018-07-02 14:14:47 PDT
Created attachment 344135 [details]
Patch for landing
Comment 5 WebKit Commit Bot 2018-07-02 15:04:13 PDT
The commit-queue encountered the following flaky tests while processing attachment 344135 [details]:

imported/blink/fast/events/click-focus-anchor-no-ring.html bug 187275 (author: jhoneycutt@apple.com)
The commit-queue is continuing to process your patch.
Comment 6 WebKit Commit Bot 2018-07-02 15:04:59 PDT
Comment on attachment 344135 [details]
Patch for landing

Clearing flags on attachment: 344135

Committed r233439: <https://trac.webkit.org/changeset/233439>
Comment 7 Radar WebKit Bug Importer 2018-07-02 15:45:24 PDT
<rdar://problem/41745021>