WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
211802
Make editing/spelling/editing-word-with-marker-2.html test what it claims and make it work on iOS
https://bugs.webkit.org/show_bug.cgi?id=211802
Summary
Make editing/spelling/editing-word-with-marker-2.html test what it claims and...
Daniel Bates
Reported
2020-05-12 13:45:36 PDT
While working on <
rdar://problem/61913405
> I realized that: 1. Test LayoutTests/editing/spelling/editing-word-with-marker-2.html was originally added to cover updating of markers when whitespace is inserted before an after a misspelling. 2. That test is skipped. 3. The test does not work on iOS. 4. That test was refactored in
bug #133544
to no longer test the "insert whitespace after misspelling" case it originally did and which it still claims to do. 5. Some other idiosyncrasies with the refactored test. 6. I can fix the test.
Attachments
Patch
(10.80 KB, patch)
2020-05-12 14:05 PDT
,
Daniel Bates
no flags
Details
Formatted Diff
Diff
Add space between sentences
(10.89 KB, patch)
2020-05-12 16:15 PDT
,
Daniel Bates
no flags
Details
Formatted Diff
Diff
Fix for anticipated WK1 failure
(10.92 KB, patch)
2020-05-12 16:18 PDT
,
Daniel Bates
no flags
Details
Formatted Diff
Diff
Attempt to fix Mac WK2
(12.62 KB, patch)
2020-05-12 16:20 PDT
,
Daniel Bates
no flags
Details
Formatted Diff
Diff
To Land
(9.96 KB, patch)
2020-05-16 09:24 PDT
,
Daniel Bates
no flags
Details
Formatted Diff
Diff
Show Obsolete
(4)
View All
Add attachment
proposed patch, testcase, etc.
Daniel Bates
Comment 1
2020-05-12 14:05:29 PDT
Created
attachment 399173
[details]
Patch
Daniel Bates
Comment 2
2020-05-12 14:10:29 PDT
Comment on
attachment 399173
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=399173&action=review
> LayoutTests/ChangeLog:13 > + The fix for iOS was simple: just enable internals.setContinuousSpellCheckingEnabled(). I didn't
^^^ + use UIHelper to activate the element in a way that brings up the keyboard, focus() alone doesn't do the due to iOS policy + blur() the field between tests for maximal code reuse (otherwise I have to keep track/check if the textarea is focused or patch up UIHelper.activateElementAndWaitForInputSession() so it doesn't hang if keyboard is already showing <-- probably a good thing to patch up, but its out of my way)
Daniel Bates
Comment 3
2020-05-12 14:11:35 PDT
Comment on
attachment 399173
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=399173&action=review
>> LayoutTests/ChangeLog:13 >> + The fix for iOS was simple: just enable internals.setContinuousSpellCheckingEnabled(). I didn't > > ^^^ + use UIHelper to activate the element in a way that brings up the keyboard, focus() alone doesn't do the due to iOS policy + blur() the field between tests for maximal code reuse (otherwise I have to keep track/check if the textarea is focused or patch up UIHelper.activateElementAndWaitForInputSession() so it doesn't hang if keyboard is already showing <-- probably a good thing to patch up, but its out of my way)
And I'm already going out of my way to fix this test...I don't need to fix this test up for what I need to do.
Darin Adler
Comment 4
2020-05-12 14:14:56 PDT
Comment on
attachment 399173
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=399173&action=review
> LayoutTests/editing/spelling/editing-word-with-marker-2.html:13 > + + "is added before or after the misspelled word or the caret is moved." > + + "The test succeeds if the word 'meagesga' has a red underline.");
Space after the period before the next sentence?
> LayoutTests/editing/spelling/editing-word-with-marker-2.html:57 > + debug(`<br>Test: ${testCase.testName}:`);
Does \n work too or do you have to use <br>?
Daniel Bates
Comment 5
2020-05-12 16:15:12 PDT
Created
attachment 399196
[details]
Add space between sentences
Daniel Bates
Comment 6
2020-05-12 16:18:02 PDT
Created
attachment 399197
[details]
Fix for anticipated WK1 failure
Daniel Bates
Comment 7
2020-05-12 16:20:12 PDT
Created
attachment 399198
[details]
Attempt to fix Mac WK2 I'm going to try, but I may choose to skip on WK2.
Daniel Bates
Comment 8
2020-05-12 16:20:57 PDT
Comment on
attachment 399173
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=399173&action=review
Thanks for the review.
>> LayoutTests/editing/spelling/editing-word-with-marker-2.html:13 >> + + "The test succeeds if the word 'meagesga' has a red underline."); > > Space after the period before the next sentence?
Fixed.
>> LayoutTests/editing/spelling/editing-word-with-marker-2.html:57 >> + debug(`<br>Test: ${testCase.testName}:`); > > Does \n work too or do you have to use <br>?
<br> required
Daniel Bates
Comment 9
2020-05-16 09:20:51 PDT
(In reply to Daniel Bates from
comment #7
)
> Created
attachment 399198
[details]
> Attempt to fix Mac WK2 > > I'm going to try, but I may choose to skip on WK2.
Experiment failed. Will do what we do now and skip the test on WK2.
Daniel Bates
Comment 10
2020-05-16 09:24:35 PDT
Created
attachment 399556
[details]
To Land
Daniel Bates
Comment 11
2020-05-16 09:25:05 PDT
Committed
r261783
: <
https://trac.webkit.org/changeset/261783
>
Radar WebKit Bug Importer
Comment 12
2020-05-16 09:26:14 PDT
<
rdar://problem/63306358
>
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