WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
REOPENED
77934
Replacement text should be available from the marker.
https://bugs.webkit.org/show_bug.cgi?id=77934
Summary
Replacement text should be available from the marker.
Hajime Morrita
Reported
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.
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
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Hajime Morrita
Comment 1
2012-02-07 17:42:17 PST
Created
attachment 125967
[details]
Patch
Hajime Morrita
Comment 2
2012-02-07 17:43:34 PST
Kent-san, Ryosuke, could you take a look? We need this for reducing round-trip over IPC.
Philippe Normand
Comment 3
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
Hajime Morrita
Comment 4
2012-02-07 20:39:25 PST
Created
attachment 125992
[details]
Patch
Kent Tamura
Comment 5
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 '-'.
Kent Tamura
Comment 6
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.
Hajime Morrita
Comment 7
2012-02-08 21:04:22 PST
Kent-san, thanks for taking a review for this! I'll address these points before landing.
Hajime Morrita
Comment 8
2012-02-08 21:23:56 PST
Committed
r107176
: <
http://trac.webkit.org/changeset/107176
>
Simon Fraser (smfr)
Comment 9
2012-09-19 13:30:31 PDT
editing/spelling/spelling-marker-description.html still fails, and is on the Skipped list.
Ahmad Saleem
Comment 10
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!
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug