We should support the inputmode attribute on contenteditable elements, in addition to <input> and <textarea>. Spec: https://html.spec.whatwg.org/multipage/interaction.html#input-modalities%3A-the-inputmode-attribute
Created attachment 347898 [details] Patch
Comment on attachment 347898 [details] Patch Attachment 347898 [details] did not pass mac-ews (mac): Output: https://webkit-queues.webkit.org/results/8954065 New failing tests: js/dom/dom-static-property-for-in-iteration.html imported/w3c/web-platform-tests/html/dom/reflection-misc.html
Created attachment 347906 [details] Archive of layout-test-results from ews103 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews103 Port: mac-sierra Platform: Mac OS X 10.12.6
Comment on attachment 347898 [details] Patch Attachment 347898 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/8954082 New failing tests: js/dom/dom-static-property-for-in-iteration.html imported/w3c/web-platform-tests/html/dom/reflection-misc.html
Created attachment 347909 [details] Archive of layout-test-results from ews104 for mac-sierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews104 Port: mac-sierra-wk2 Platform: Mac OS X 10.12.6
Comment on attachment 347898 [details] Patch Attachment 347898 [details] did not pass mac-debug-ews (mac): Output: https://webkit-queues.webkit.org/results/8954095 New failing tests: js/dom/dom-static-property-for-in-iteration.html
Created attachment 347912 [details] Archive of layout-test-results from ews116 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews116 Port: mac-sierra Platform: Mac OS X 10.12.6
Comment on attachment 347898 [details] Patch Attachment 347898 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: https://webkit-queues.webkit.org/results/8954197 New failing tests: imported/w3c/web-platform-tests/html/dom/reflection-misc.html
Created attachment 347914 [details] Archive of layout-test-results from ews121 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews121 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.4
Comment on attachment 347898 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=347898&action=review > Source/WebCore/html/HTMLElement.h:113 > + const AtomicString& inputMode() const; > + void setInputMode(const AtomicString& value); I don't think it makes much sense for these functions to take & return AtomicString since setInputMode does a case-insensitive comparison. Just use String instead. > LayoutTests/fast/forms/inputmode-attribute.html:29 > +shouldBe('textarea.inputMode = "tel"; textarea.inputMode', '"tel"'); > +shouldBe('textarea.getAttribute("inputmode")', '"tel"'); > +shouldBe('textarea.setAttribute("inputmode", "tel"); textarea.inputMode', '"tel"'); > +shouldBe('editor.inputMode = "url"; editor.inputMode', '"url"'); We need to test every type on every element type. r- because of this reduces the test coverage and doesn't provide adequate amount of testing for content editable element support.
Comment on attachment 347898 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=347898&action=review >> Source/WebCore/html/HTMLElement.h:113 >> + void setInputMode(const AtomicString& value); > > I don't think it makes much sense for these functions to take & return AtomicString > since setInputMode does a case-insensitive comparison. Just use String instead. (See: <https://bugs.webkit.org/show_bug.cgi?id=183621#c13>)
(In reply to Ryosuke Niwa from comment #10) > Comment on attachment 347898 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=347898&action=review > > > Source/WebCore/html/HTMLElement.h:113 > > + const AtomicString& inputMode() const; > > + void setInputMode(const AtomicString& value); > > I don't think it makes much sense for these functions to take & return > AtomicString > since setInputMode does a case-insensitive comparison. Just use String > instead. > I take it you meant to say that inputMode() does a case-insensitive comparison. Regardless, having either one of these functions (or both) take or return a String requires conversion (though it may be implicit and even optimized away). It is most natural for both of these functions to operate on- and return a- const lvalue reference to an AtomicString. The setter, setInputMode(), turns around and calls Element::setAttributeWithoutSynchronization() passing the new value and Element::setAttributeWithoutSynchronization() expects an AtomicString; => the setter needs to pay the conversion cost to AtomicString anyway; => the setter should take a const AtomicString&. For the getter, inputMode(), it is most natural (i.e. avoids conversion) to return a const AtomicString& because all of the WebCore::InputModeNames::*() (e.g. InputModeNames::text()) return a const AtomicString&. Therefore, it is most natural for both the setter and getter to take and return a const AtomicString&.
Created attachment 347943 [details] Patch
(In reply to Ryosuke Niwa from comment #10) > Comment on attachment 347898 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=347898&action=review > > > LayoutTests/fast/forms/inputmode-attribute.html:29 > > +shouldBe('textarea.inputMode = "tel"; textarea.inputMode', '"tel"'); > > +shouldBe('textarea.getAttribute("inputmode")', '"tel"'); > > +shouldBe('textarea.setAttribute("inputmode", "tel"); textarea.inputMode', '"tel"'); > > +shouldBe('editor.inputMode = "url"; editor.inputMode', '"url"'); > > We need to test every type on every element type. > r- because of this reduces the test coverage and doesn't provide adequate > amount of testing for content editable element support. Added additional tests to increase test coverage.
Comment on attachment 347943 [details] Patch Clearing flags on attachment: 347943 Committed r235245: <https://trac.webkit.org/changeset/235245>
All reviewed patches have been landed. Closing bug.
<rdar://problem/43658106>
This change broke the Windows build: c:\cygwin\home\buildbot\slave\win-release\build\source\webcore\html\inputmode.cpp(73): error C2220: warning treated as error - no 'object' file generated [C:\cygwin\home\buildbot\slave\win-release\build\WebKitBuild\Release\Source\WebCore\WebCore.vcxproj] c:\cygwin\home\buildbot\slave\win-release\build\source\webcore\html\inputmode.cpp(73): warning C4715: 'WebCore::stringForInputMode': not all control paths return a value [C:\cygwin\home\buildbot\slave\win-release\build\WebKitBuild\Release\Source\WebCore\WebCore.vcxproj] https://build.webkit.org/builders/Apple%20Win%20Release%20%28Build%29/builds/11402
Committed r235263 as a fix.