Bug 77934 - Replacement text should be available from the marker.
Summary: Replacement text should be available from the marker.
Status: REOPENED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Hajime Morrita
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-02-06 22:11 PST by Hajime Morrita
Modified: 2022-10-05 16:04 PDT (History)
8 users (show)

See Also:


Attachments
Patch (18.11 KB, patch)
2012-02-07 17:42 PST, Hajime Morrita
no flags Details | Formatted Diff | Diff
Patch (22.46 KB, patch)
2012-02-07 20:39 PST, Hajime Morrita
tkent: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Hajime Morrita 2012-02-06 22:11:56 PST
On spellchecking, TextCheckingResult contains a replacement text
which can be used both for an automatic replacement and for showing a suggestion.

But when marking a misspelled word, Editor uses only the misspelled range data
but discards the replacement value. Then it asks the same value again
when showing suggestion/autocorrection.

It would be great if the marker holds the replacement text
and Editor can use it on suggesting a correction, without any re-asking.
This is especially true in the case when we need IPC messaging to spelling correction:
We can save one roundtrip by doing this.
Comment 1 Hajime Morrita 2012-02-07 17:42:17 PST
Created attachment 125967 [details]
Patch
Comment 2 Hajime Morrita 2012-02-07 17:43:34 PST
Kent-san, Ryosuke, could you take a look? We need this for reducing round-trip over IPC.
Comment 3 Philippe Normand 2012-02-07 18:18:09 PST
Comment on attachment 125967 [details]
Patch

Attachment 125967 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/11459208
Comment 4 Hajime Morrita 2012-02-07 20:39:25 PST
Created attachment 125992 [details]
Patch
Comment 5 Kent Tamura 2012-02-08 20:24:21 PST
Comment on attachment 125992 [details]
Patch

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

> Source/WebCore/testing/Internals.h:84
> +    String markerDescriptionForNode(Node*, const String&, unsigned, ExceptionCode&);

nit: We shouldn't omit the argument names of 2nd and 3rd arguments. It's hard to understand what they are.

> Source/WebCore/testing/Internals.h:118
> +    DocumentMarker* markerAt(Node*, const String&, unsigned, ExceptionCode&);

ditto.

> LayoutTests/editing/spelling/spelling-marker-description.html:43
> +    window.setTimeout(function() { verify(nretry-1); }, 0);

nit: Should have whitespace around '-'.
Comment 6 Kent Tamura 2012-02-08 20:25:08 PST
(In reply to comment #0)
> On spellchecking, TextCheckingResult contains a replacement text
> which can be used both for an automatic replacement and for showing a suggestion.
> 
> But when marking a misspelled word, Editor uses only the misspelled range data
> but discards the replacement value. Then it asks the same value again
> when showing suggestion/autocorrection.
> 
> It would be great if the marker holds the replacement text
> and Editor can use it on suggesting a correction, without any re-asking.
> This is especially true in the case when we need IPC messaging to spelling correction:
> We can save one roundtrip by doing this.

It would be helpful if ChangeLog has such information.
Comment 7 Hajime Morrita 2012-02-08 21:04:22 PST
Kent-san, thanks for taking a review for this! I'll address these points before landing.
Comment 8 Hajime Morrita 2012-02-08 21:23:56 PST
Committed r107176: <http://trac.webkit.org/changeset/107176>
Comment 9 Simon Fraser (smfr) 2012-09-19 13:30:31 PDT
editing/spelling/spelling-marker-description.html still fails, and is on the Skipped list.
Comment 10 Ahmad Saleem 2022-10-05 16:04:32 PDT
(In reply to Simon Fraser (smfr) from comment #9)
> editing/spelling/spelling-marker-description.html still fails, and is on the
> Skipped list.

Still on SKIP list:

https://github.com/WebKit/WebKit/blob/95606b13126e82a5cf2534a05e107d0aa69ccc0a/LayoutTests/platform/win/TestExpectations#L1610

and this seems to have landed in this commit:

https://github.com/WebKit/WebKit/commit/7c6135b1f0fb885d404a2e8678715cc5dbf6747e

Since original bug is fixed, can we mark this as "RESOLVED FIXED" and for skipped test, we have another bug? Thanks!