Bug 185584 - [macOS] Spelling errors in the middle of an inserted paragraph are not displayed
Summary: [macOS] Spelling errors in the middle of an inserted paragraph are not displayed
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-05-12 21:11 PDT by Wenson Hsieh
Modified: 2018-06-06 09:59 PDT (History)
7 users (show)

See Also:


Attachments
Patch (26.16 KB, patch)
2018-06-05 13:16 PDT, Wenson Hsieh
rniwa: review+
Details | Formatted Diff | Diff
Patch for landing (25.96 KB, patch)
2018-06-05 18:05 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-05-12 21:11:13 PDT
Spellcheck results in the middle of an inserted paragraph of text are not displayed after ending the paragraph.

To reproduce:

1. Paste "Helol wrodl" into a contenteditable div or textarea.
2. Hit enter to insert a newline

Expected: red underlines underneath "Helol" and "wrodl".
Observed: red underlines only underneath "wrodl".

Reproduces on macOS El Capitan using system Safari (possibly reproduces earlier than El Capitan).
Comment 1 Wenson Hsieh 2018-05-12 23:58:02 PDT
<rdar://problem/38676081>
Comment 2 Wenson Hsieh 2018-06-05 13:16:52 PDT
Created attachment 341987 [details]
Patch
Comment 3 Ryosuke Niwa 2018-06-05 17:45:41 PDT
Comment on attachment 341987 [details]
Patch

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

> Source/WebCore/ChangeLog:25
> +        non-contiguous spelling correction ranges â see the FIXME in markMisspellingsAfterTypingToWord for more detail.

Did you mean to put a period between ranges & see?

> Source/WebCore/editing/Editor.cpp:2388
> +        while (sentenceStart < spellCheckingStart) {

Should we be running this code in old macOS too? Does that even work?

> LayoutTests/editing/spelling/retro-correction-spelling-markers.html:11
> +        document.getElementById("description").innerHTML = `To manually test, type <strong>It's muhc to late

Hm.. I think typeCharacterCommand should trigger spell checking in the browser too.
Comment 4 Wenson Hsieh 2018-06-05 17:57:59 PDT
Comment on attachment 341987 [details]
Patch

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

>> Source/WebCore/ChangeLog:25
>> +        non-contiguous spelling correction ranges â see the FIXME in markMisspellingsAfterTypingToWord for more detail.
> 
> Did you mean to put a period between ranges & see?

I put an emdash (—), but it seems Bugzilla's diff viewer doesn't handle unicode well :/

>> Source/WebCore/editing/Editor.cpp:2388
>> +        while (sentenceStart < spellCheckingStart) {
> 
> Should we be running this code in old macOS too? Does that even work?

Yes, I believe so. This fixes an existing bug wherein pasting some text that contains misspelled words will not add document markers for the misspelled ranges. This case is covered by one of the new tests, spelling-markers-after-pasting-sentence.html.

>> LayoutTests/editing/spelling/retro-correction-spelling-markers.html:11
>> +        document.getElementById("description").innerHTML = `To manually test, type <strong>It's muhc to late
> 
> Hm.. I think typeCharacterCommand should trigger spell checking in the browser too.

Oh, that's a good point! I'll make it so that we automatically type out the string when opening this manually.
Comment 5 Wenson Hsieh 2018-06-05 18:05:28 PDT
Created attachment 342011 [details]
Patch for landing
Comment 6 WebKit Commit Bot 2018-06-05 18:44:09 PDT
Comment on attachment 342011 [details]
Patch for landing

Clearing flags on attachment: 342011

Committed r232530: <https://trac.webkit.org/changeset/232530>