Summary: | WKBundleNodeHandleSetHTMLInputElementSpellcheckEnabled should keep text replacement enabled | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Ryosuke Niwa <rniwa> | ||||||||||||
Component: | HTML Editing | Assignee: | Ryosuke Niwa <rniwa> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | andersca, buildbot, enrica, jberlin, rniwa, wenson_hsieh | ||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||
Hardware: | Unspecified | ||||||||||||||
OS: | Unspecified | ||||||||||||||
Attachments: |
|
Description
Ryosuke Niwa
2016-11-16 23:02:24 PST
Created attachment 295032 [details]
Fixes the bug
Created attachment 295034 [details]
Leave smart quotes/dashes disabled
Comment on attachment 295034 [details] Leave smart quotes/dashes disabled Attachment 295034 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/2530138 New failing tests: editing/spelling/spellcheck-attribute.html editing/spelling/spellcheck-paste.html editing/spelling/spelling-attribute-change.html editing/spelling/spelling-hasspellingmarker.html editing/spelling/spellcheck-queue.html Created attachment 295040 [details]
Archive of layout-test-results from ews102 for mac-yosemite
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102 Port: mac-yosemite Platform: Mac OS X 10.10.5
Comment on attachment 295034 [details] Leave smart quotes/dashes disabled Attachment 295034 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/2530143 New failing tests: editing/spelling/spelling-attribute-change.html editing/spelling/spellcheck-queue.html editing/spelling/spellcheck-attribute.html editing/spelling/spelling-hasspellingmarker.html editing/spelling/spellcheck-paste.html Created attachment 295041 [details]
Archive of layout-test-results from ews115 for mac-yosemite
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115 Port: mac-yosemite Platform: Mac OS X 10.10.5
Comment on attachment 295034 [details] Leave smart quotes/dashes disabled View in context: https://bugs.webkit.org/attachment.cgi?id=295034&action=review > Source/WebCore/ChangeLog:15 > + No new tests since we don't have a good facility to test text replacement. By text replacement, do you mean selecting a candidate in the popover that appears over a word to replace it? I believe internals.handleAcceptedCandidate exercises this codepath. Comment on attachment 295034 [details]
Leave smart quotes/dashes disabled
Waiting to review until the patch passes tests on EWS and until we verify that internals.handleAcceptedCandidate can’t be used to add another test.
(In reply to comment #8) > Comment on attachment 295034 [details] > Leave smart quotes/dashes disabled > > View in context: > https://bugs.webkit.org/attachment.cgi?id=295034&action=review > > > Source/WebCore/ChangeLog:15 > > + No new tests since we don't have a good facility to test text replacement. > > By text replacement, do you mean selecting a candidate in the popover that > appears over a word to replace it? I believe > internals.handleAcceptedCandidate exercises this codepath. No, this is a feature enabled by System Preference > Keyboard > Text. What this patch does is surpassing spellchecking requests but allowing text replacement feature so I don’t think we can use that internals method. We likely need some sort of a new WebKitTestRunner feature to do this if anything. Comment on attachment 295034 [details] Leave smart quotes/dashes disabled View in context: https://bugs.webkit.org/attachment.cgi?id=295034&action=review > Source/WebCore/html/HTMLInputElement.cpp:119 > - , m_isSpellCheckingEnabled(true) > + , m_isSpellcheckDisabledExceptTextReplacement(true) The tests are failing because this needs to be false. Created attachment 295109 [details]
Fixed the tests
Comment on attachment 295109 [details] Fixed the tests View in context: https://bugs.webkit.org/attachment.cgi?id=295109&action=review > Source/WebCore/editing/Editor.cpp:3487 > +TextCheckingTypeMask Editor::resolveTextCheckingTypeMask(Node& rootEditableElement, TextCheckingTypeMask textCheckingOptions) Can this be a const Node& rootEditableElement? Thanks for the review! (In reply to comment #13) > Comment on attachment 295109 [details] > Fixed the tests > > View in context: > https://bugs.webkit.org/attachment.cgi?id=295109&action=review > > > Source/WebCore/editing/Editor.cpp:3487 > > +TextCheckingTypeMask Editor::resolveTextCheckingTypeMask(Node& rootEditableElement, TextCheckingTypeMask textCheckingOptions) > > Can this be a const Node& rootEditableElement? Yup, fixed. Committed r208875: <http://trac.webkit.org/changeset/208875> |