inputmode attribute support should be added to comply with: http://www.w3.org/2008/06/mobile-test/ The attribute is defined and specified here: http://www.w3.org/TR/xhtml-basic
Created attachment 27108 [details] Patch from George to implement Implements the basic skeleton for inputmode support. The UI elements come next.
Comment on attachment 27108 [details] Patch from George to implement I'd be interested to hear more about what the rest of the support would look like. Just adding this js accessor (especially if just to pass http://www.w3.org/2008/06/mobile-test/) seems silly. Adding real inputmethod= support seems interesting, but I'm not sure what that support would look like. Maybe you can tell me more?
We plan to implement all of the rest in our application.
Any reviewer?
Comment on attachment 27108 [details] Patch from George to implement I don't think it is appropriate to unconditionally include this attribute as it will make feature detection incorrect for embedders who do not implement this functionality or do not want to. That said, I am not sure what the correct #define would be.
ENABLE(INPUTMODE)?
Created attachment 58364 [details] patch for XHTML Basic 1.1 This patch is to add the XHTML Basic 1.1 support (conditionally with ENABLE_XHTMLBASIC), specifically, inputmethod support. Attribute "inputmethod" for "input" and "textarea" element servers as a input method hint, so that we the control is in focus, the system automatically switchs to the input method indicated by "inputmethod" attribute. This applies to most mobile devices which have limited input hardware, and need to switch between input methods to take advantage of limited hardware input capabilities. The inputmode helps to facilitate the input method switching, otherwise, the user has to manually switches to the right input method. The inputmode is only a "hint", or suggestion, for the input method, it doesn't prevent the user from switching to other input methods manually, as they like, and it doesn't validate the acutally input from the user. The patch addresses the WebCore part of this feature, and each porting need to implement a platform-specific interface to actually switch the input method, this can be done in an existing interface: EditorClient::setInputMethodState(bool). Before this patch, EditorClient::setInputMethodState(bool) only enables/disables the input method, automatically when the focus is on text node / non-text node. The implementation of this API should also be responsible for switching to the right Input Method based on the inputmode value, so we are not going to introduce a new API for EditorClient. The patch contains a reference code in for QT, other portings can do similar to switch the Input Methods based on inputmode value.
Some comments in passing: +contains(DEFINES, ENABLE_XHTMLBASIC=1): FEATURE_DEFINES_JAVASCRIPT += ENABLE_XHTMLBASIC=1 The feature should be implemented in a way that it either works, or is undetectable from JavaScript - even if someone mistakenly enables the feature define, this invariant shouldn't be broken. +// The code is here for reference purpose, each platform should implement the input mode switching based on +// the platform capability and the inputmode attribute, like digit, upperCase, lowerCase, or any language-specific +// input methods + +#if 0 We don't normally land commented out code. And implementation for a specific platform is not a place where one would easily find such explanation. + Need a short description and bug URL (OOPS!) It does! > Index: LayoutTests/platform/qt/fast/xhtmlbasic/xhtmlbasic11-expected.txt Is there a reason for the test to not be dumpAsText?
Comment on attachment 58364 [details] patch for XHTML Basic 1.1 Actually, let me turn the above into a formal review, r-.
> +contains(DEFINES, ENABLE_XHTMLBASIC=1): FEATURE_DEFINES_JAVASCRIPT += ENABLE_XHTMLBASIC=1 > > The feature should be implemented in a way that it either works, or is undetectable from JavaScript - even if someone mistakenly enables the feature define, this invariant shouldn't be broken. > Alexey, thanks for reviewing . Will you please elaborate what you mean with above comments? As I said in above comments, this patch only addresses the WebCore part of this feature, and each porting need to have the platform/device specific implementation for input-method switching. Anybody who enables this feature with ENABLE_XHTMLBASIC need to have their own implementation to get it working. But without this patch, they hve no way to get it working.
*** Bug 16178 has been marked as a duplicate of this bug. ***
*** Bug 16486 has been marked as a duplicate of this bug. ***
Created attachment 58440 [details] Resubmission after addressing reviewer's comments 1. Reference platform-specific implementation of EditorClientQT::setInputModeStatus(bool) removed, leave it for platform/device implementations. 2. test case xhtmlbasic11.xhtml used dumpAsText 3. ChangeLog comments added for WebKitTools/ChangeLog
Comment on attachment 58440 [details] Resubmission after addressing reviewer's comments > Index: WebCore/ChangeLog > =================================================================== > --- WebCore/ChangeLog (revision 60986) > +++ WebCore/ChangeLog (working copy) > @@ -1,3 +1,22 @@ > +2010-06-10 Charles Wei <charles.wei@torchmobile.com.cn> > + > + Reviewed by NOBODY (OOPS!). > + > + Add XHTML Basic 1.1 support for "inputmode" attribute for "input" and "textarea" element > + https://bugs.webkit.org/show_bug.cgi?id=23588 > + > + Tests: fast/xhtmlbasic/inputmode.xhtml > + fast/xhtmlbasic/xhtmlbasic11.xhtml > + > + * WebCore.pri: > + * html/HTMLAttributeNames.in: > + * html/HTMLFormControlElement.cpp: > + (WebCore::HTMLTextFormControlElement::inputmode): > + (WebCore::HTMLTextFormControlElement::setInputmode): > + * html/HTMLFormControlElement.h: > + * html/HTMLInputElement.idl: > + * html/HTMLTextAreaElement.idl: > + > 2010-06-10 Tony Chang <tony@chromium.org> > > Reviewed by Kent Tamura. > Index: WebCore/WebCore.pri > =================================================================== > --- WebCore/WebCore.pri (revision 60986) > +++ WebCore/WebCore.pri (working copy) > @@ -159,6 +159,7 @@ contains(DEFINES, ENABLE_FILTERS=1): FEA > contains(DEFINES, ENABLE_WCSS=1): FEATURE_DEFINES_JAVASCRIPT += ENABLE_WCSS=1 > contains(DEFINES, ENABLE_WML=1): FEATURE_DEFINES_JAVASCRIPT += ENABLE_WML=1 > contains(DEFINES, ENABLE_XHTMLMP=1): FEATURE_DEFINES_JAVASCRIPT += ENABLE_XHTMLMP=1 > +contains(DEFINES, ENABLE_XHTMLBASIC=1): FEATURE_DEFINES_JAVASCRIPT += ENABLE_XHTMLBASIC=1 > contains(DEFINES, ENABLE_SVG=1): FEATURE_DEFINES_JAVASCRIPT += ENABLE_SVG=1 > contains(DEFINES, ENABLE_JAVASCRIPT_DEBUGGER=1): FEATURE_DEFINES_JAVASCRIPT += ENABLE_JAVASCRIPT_DEBUGGER=1 > contains(DEFINES, ENABLE_OFFLINE_WEB_APPLICATIONS=1): FEATURE_DEFINES_JAVASCRIPT += ENABLE_OFFLINE_WEB_APPLICATIONS=1 > Index: WebCore/html/HTMLAttributeNames.in > =================================================================== > --- WebCore/html/HTMLAttributeNames.in (revision 60986) > +++ WebCore/html/HTMLAttributeNames.in (working copy) > @@ -107,6 +107,7 @@ http_equiv > id > incremental > indeterminate > +inputmode > ismap > keytype > label > Index: WebCore/html/HTMLFormControlElement.cpp > =================================================================== > --- WebCore/html/HTMLFormControlElement.cpp (revision 60986) > +++ WebCore/html/HTMLFormControlElement.cpp (working copy) > @@ -637,4 +637,16 @@ void HTMLTextFormControlElement::parseMa > HTMLFormControlElementWithState::parseMappedAttribute(attr); > } > > +#if ENABLE(XHTMLBASIC) > +String HTMLTextFormControlElement::inputmode() const > +{ > + return getAttribute(inputmodeAttr).string(); > +} > + > +void HTMLTextFormControlElement::setInputmode(const String& value) > +{ > + setAttribute(inputmodeAttr, value); > +} > +#endif > + > } // namespace Webcore > Index: WebCore/html/HTMLFormControlElement.h > =================================================================== > --- WebCore/html/HTMLFormControlElement.h (revision 60986) > +++ WebCore/html/HTMLFormControlElement.h (working copy) > @@ -189,6 +189,13 @@ public: > void setSelectionRange(int start, int end); > VisibleSelection selection() const; > > +#if ENABLE(XHTMLBASIC) > + virtual String value() const = 0; > + String inputmode() const; > + void setInputmode(const String&); > +#endif > + > + > protected: > HTMLTextFormControlElement(const QualifiedName&, Document*, HTMLFormElement*); > > Index: WebCore/html/HTMLInputElement.idl > =================================================================== > --- WebCore/html/HTMLInputElement.idl (revision 60986) > +++ WebCore/html/HTMLInputElement.idl (working copy) > @@ -98,6 +98,11 @@ module html { > > readonly attribute FileList files; > readonly attribute NodeList labels; > + > +#if defined(ENABLE_XHTMLBASIC) && ENABLE_XHTMLBASIC > + attribute [ConvertNullToNullString] DOMString inputmode; > +#endif > + > }; > > } > Index: WebCore/html/HTMLTextAreaElement.idl > =================================================================== > --- WebCore/html/HTMLTextAreaElement.idl (revision 60986) > +++ WebCore/html/HTMLTextAreaElement.idl (working copy) > @@ -51,6 +51,10 @@ module html { > attribute long selectionEnd; > void setSelectionRange(in long start, in long end); > readonly attribute NodeList labels; > + > +#if defined(ENABLE_XHTMLBASIC) && ENABLE_XHTMLBASIC > + attribute [ConvertNullToNullString] DOMString inputmode; > +#endif > }; > > } > Index: WebKitTools/ChangeLog > =================================================================== > --- WebKitTools/ChangeLog (revision 60986) > +++ WebKitTools/ChangeLog (working copy) > @@ -1,3 +1,14 @@ > +2010-06-10 Charles Wei <charles.wei@torchmobile.com.cn> > + > + Reviewed by NOBODY (OOPS!). > + > + Add feature "XHTML Basic 1.1 support with ENABLE_XHTMLBASIC > + https://bugs.webkit.org/show_bug.cgi?id=23588 > + > + * Scripts/build-webkit: > + * Scripts/old-run-webkit-tests: > + * Scripts/webkitperl/features.pm: > + > 2010-06-10 Ojan Vafai <ojan@chromium.org> > > Reviewed by Alexey Proskuryakov. > Index: WebKitTools/Scripts/build-webkit > =================================================================== > --- WebKitTools/Scripts/build-webkit (revision 60986) > +++ WebKitTools/Scripts/build-webkit (working copy) > @@ -59,7 +59,7 @@ my ($threeDCanvasSupport, $threeDRenderi > $domStorageSupport, $eventsourceSupport, $filtersSupport, $geolocationSupport, $iconDatabaseSupport, $imageResizerSupport, $indexedDatabaseSupport, > $javaScriptDebuggerSupport, $mathmlSupport, $offlineWebApplicationSupport, $rubySupport, $systemMallocSupport, $sandboxSupport, $sharedWorkersSupport, > $svgSupport, $svgAnimationSupport, $svgAsImageSupport, $svgDOMObjCBindingsSupport, $svgFontsSupport, > - $svgForeignObjectSupport, $svgUseSupport, $videoSupport, $webSocketsSupport, $wmlSupport, $wcssSupport, $xhtmlmpSupport, $workersSupport, > + $svgForeignObjectSupport, $svgUseSupport, $videoSupport, $webSocketsSupport, $wmlSupport, $wcssSupport, $xhtmlmpSupport, $xhtmlbasicSupport, $workersSupport, > $xpathSupport, $xsltSupport, $coverageSupport, $notificationsSupport, $blobSliceSupport, $tiledBackingStoreSupport, > $fileReaderSupport, $fileWriterSupport); > > @@ -172,6 +172,9 @@ my @features = ( > { option => "xhtmlmp", desc => "Toggle XHTML-MP support", > define => "ENABLE_XHTMLMP", default => 0, value => \$xhtmlmpSupport }, > > + { option => "xhtmlbasic", desc => "Toggle XHTML Basic 1.1 support", > + define => "ENABLE_XHTMLBASIC", default => 0, value => \$xhtmlbasicSupport }, > + > { option => "wcss", desc => "Toggle WCSS support", > define => "ENABLE_WCSS", default => 0, value => \$wcssSupport }, > > Index: WebKitTools/Scripts/old-run-webkit-tests > =================================================================== > --- WebKitTools/Scripts/old-run-webkit-tests (revision 60986) > +++ WebKitTools/Scripts/old-run-webkit-tests (working copy) > @@ -518,6 +518,10 @@ if (!checkWebCoreFeatureSupport("WCSS", > $ignoredDirectories{'fast/wcss'} = 1; > } > > +if (!checkWebCoreFeatureSupport("XHTMLBASIC", 0)) { > + $ignoredDirectories{'fast/xhtmlbasic'} = 1; > +} > + > if (!checkWebCoreFeatureSupport("XHTMLMP", 0)) { > $ignoredDirectories{'fast/xhtmlmp'} = 1; > } > Index: WebKitTools/Scripts/webkitperl/features.pm > =================================================================== > --- WebKitTools/Scripts/webkitperl/features.pm (revision 60986) > +++ WebKitTools/Scripts/webkitperl/features.pm (working copy) > @@ -74,6 +74,7 @@ sub hasFeature($$) > "WML" => "WMLElement", > "WCSS" => "parseWCSSInputProperty", > "XHTMLMP" => "isXHTMLMPDocument", > + "XHTMLBASIC" => "setInputmode", > ); > my $symbolName = $symbolForFeature{$featureName}; > die "Unknown feature: $featureName" unless $symbolName; > Index: LayoutTests/ChangeLog > =================================================================== > --- LayoutTests/ChangeLog (revision 60986) > +++ LayoutTests/ChangeLog (working copy) > @@ -1,3 +1,16 @@ > +2010-06-10 Charles Wei <charles.wei@torchmobile.com.cn> > + > + Reviewed by NOBODY (OOPS!). > + > + Add test cases for XHTML Basic 1.1 > + https://bugs.webkit.org/show_bug.cgi?id=23588 > + > + * fast/xhtmlbasic: Added. > + * fast/xhtmlbasic/inputmode-expected.txt: Added. > + * fast/xhtmlbasic/inputmode.xhtml: Added. > + * fast/xhtmlbasic/xhtmlbasic11-expected.txt: Added. > + * fast/xhtmlbasic/xhtmlbasic11.xhtml: Added. > + > 2010-06-10 Tony Chang <tony@chromium.org> > > Reviewed by Kent Tamura. > Index: LayoutTests/fast/xhtmlbasic/inputmode-expected.txt > =================================================================== > --- LayoutTests/fast/xhtmlbasic/inputmode-expected.txt (revision 0) > +++ LayoutTests/fast/xhtmlbasic/inputmode-expected.txt (revision 0) > @@ -0,0 +1,13 @@ > + > +Verifies initial inputmode for input is 'digit' : > +PASS document.getElementById("input").inputmode is "digit" > + > +Verifies inputmode for input is changed to 'latin' : > +PASS document.getElementById("input").inputmode is "latin" > + > +Verifies initial inputmode for textarea is 'upperCase' : > +PASS document.getElementById("textarea").inputmode is "upperCase" > + > +Verifies inputmode for textarea is changed to 'lowerCase' : > +PASS document.getElementById("textarea").inputmode is "lowerCase" > + > Index: LayoutTests/fast/xhtmlbasic/inputmode.xhtml > =================================================================== > --- LayoutTests/fast/xhtmlbasic/inputmode.xhtml (revision 0) > +++ LayoutTests/fast/xhtmlbasic/inputmode.xhtml (revision 0) > @@ -0,0 +1,42 @@ > +<?xml version="1.0"?> > +<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML Basic 1.1//EN" "http://www.w3.org/TR/xhtml-basic/xhtml-basic11.dtd"> > +<html xmlns="http://www.w3.org/1999/xhtml"> > +<head> > +<script src="../js/resources/js-test-pre.js"></script> > +</head> > + > +<body> > +<input type="text" id="input" inputmode="digit" /> > +<textarea id="textarea" inputmode="upperCase" /> > + > +<p id="console"> </p> > + > +<script language="JavaScript" type="text/javascript"> > + > +function log(message) { > + document.getElementById("console").innerHTML += message ; > +} > + if (window.layoutTestController) > + layoutTestController.dumpAsText(); > + > + debug("Verifies initial inputmode for input is 'digit' : " ); > + shouldBe('document.getElementById("input").inputmode' , '"digit"'); > + debug(""); > + > + document.getElementById("input").inputmode = "latin"; > + debug("Verifies inputmode for input is changed to 'latin' : " ); > + shouldBe('document.getElementById("input").inputmode', '"latin"' ); > + debug(""); > + > + debug("Verifies initial inputmode for textarea is 'upperCase' : "); > + shouldBe('document.getElementById("textarea").inputmode', '"upperCase"'); > + debug(""); > + > + document.getElementById("textarea").inputmode = "lowerCase"; > + debug("Verifies inputmode for textarea is changed to 'lowerCase' : "); > + shouldBe('document.getElementById("textarea").inputmode', '"lowerCase"'); > + > +</script> > + > +</body> > +</html> > Index: LayoutTests/fast/xhtmlbasic/xhtmlbasic11-expected.txt > =================================================================== > --- LayoutTests/fast/xhtmlbasic/xhtmlbasic11-expected.txt (revision 0) > +++ LayoutTests/fast/xhtmlbasic/xhtmlbasic11-expected.txt (revision 0) > @@ -0,0 +1,3 @@ > +This test case verifies XHTML Basic 1.1 DOCTYPE is correctly parsed and rendered. > + > +PASS > Index: LayoutTests/fast/xhtmlbasic/xhtmlbasic11.xhtml > =================================================================== > --- LayoutTests/fast/xhtmlbasic/xhtmlbasic11.xhtml (revision 0) > +++ LayoutTests/fast/xhtmlbasic/xhtmlbasic11.xhtml (revision 0) > @@ -0,0 +1,19 @@ > +<?xml version="1.0"?> > +<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML Basic 1.1//EN" "http://www.w3.org/TR/xhtml-basic/xhtml-basic11.dtd"> > + > +<html xmlns="http://www.w3.org/1999/xhtml"> > +<head> > +<title>Test document conformance of XHTML Basic 1.1 </title> > +</head> > +<body> > +<p> This test case verifies XHTML Basic 1.1 DOCTYPE is correctly parsed and rendered. </p> > +<p id="console"> </p> > +<script type="text/javascript"> > + if (window.layoutTestController){ > + layoutTestController.dumpAsText(); > + } > + document.getElementById("console").innerHTML = "PASS"; > + > +</script> > +</body> > +</html> WebCore/html/HTMLFormControlElement.h:195 + void setInputmode(const String&); Are inputmode() and setInputmode() really needed? We can omit them by adding [Reflect] in IDL files, and platform-dependent code can access them by getAttribute() and setAttribute(). LayoutTests/ChangeLog:12 + * fast/xhtmlbasic/xhtmlbasic11.xhtml: Added. We should add these tests to Skipped files of unsupported platforms.
Oops, I'm sorry for not removing the entire patch from the comment.
Created attachment 58455 [details] Resubmission after addressing reviewer's comments Thanks Kent for the review . Resubmitting following Kent's comments, with the following comments: 1. Removed the HTMLTextFormControlElement::inputmode() and HTMLTextFormControlElement::setInputmode(), and add modifier "Reflect" in the idl file of HTMLInputElement and HTMLTextAreaInput so that we don't need to define inputmode() and setInputmode() 2. The test cases will be automatically skipped by unsupported platforms, we don't need to add them to skip files.
Comment on attachment 58455 [details] Resubmission after addressing reviewer's comments You need to change more files to introduce a feature flag. e.g. */FeatureDefines.xcconfig, features.gypi. Bug#40878 has a good example. WebCore/html/HTMLInputElement.idl:103 + attribute [ConvertNullToNullString, Reflect] DOMString inputmode; You don't need ConvertNullToNullString anymore. Reflect implies it. WebCore/html/HTMLTextAreaElement.idl:56 + attribute [ConvertNullToNullString, Reflect] DOMString inputmode; ditto. WebKitTools/ChangeLog:5 + Add feature "XHTML Basic 1.1 support with ENABLE_XHTMLBASIC Unclosed "
Should this be implemented at all given that it is not in HTML5?
(In reply to comment #18) > Should this be implemented at all given that it is not in HTML5? Also, I don't know of any other deployed UA that actually supports the inputmode attribute. So, another thing to consider is whether there's actually any existing content (even WAP/mobile-only content) that uses the inputmode attribute. It would seem like there would not be -- since there's no other UA that supports inputmode. But if this starts shipping in Webkit-based browsers, then it is likely that some content providers will start using it. I think we should be carefully considering whether we want that to happen -- because then other UA vendors will then be expected to add support for it as well, and incur development and testing costs in order to do that. So the question is whether inputmode is important enough that all UA vendors should add support for it. An important thing to note is that inputmode essentially duplicates functionality that's already provided in most deployed mobile UAs through other means -- chiefly through the "-wap-input-format", which has for better or worse become a de facto standard, and is (as I understand it) already supported in Webkit and in Opera and in all modern WAP browsers. And the -wap-input-format is already used in a huge amount of existing (mobile-only/mobile-friendly) content. So to some degree, the inputmode is reinventing a wheel in spite of that fact that there's no actually no market need/demand for it to be reinvented. All that said, I'll concede that I am not really up to date with details of the current market situation around mobile browsers, so it could be that what I have written above no longer applies.
-wap-input-format isn't actually supported by most Webkit mobile browsers as far as I know; it isn't supported on the iPhone for sure. Currently, the iPhone relies on form control names convention (e.g. including phone or zip in the name of the field) to trigger the numerical virtual keypad, which seems suboptimal at best. I think the inputmode attribute has clearly a role to play to improve keyboard input on mobile devices, esp. with the rise of virtual keyboards. It used to be part of HTML5, but was removed due to lack of implementations (from what I remember) — so I'm hoping a proposal to implement it won't be blocked by its lack of inclusion in HTML5! From what I know, Opera has the infrastructure to implement the inputmode attribute (i.e. it is properly parsed as part of the DOM), but hasn't exposed it in its deployed browsers (yet?). To summarize, I really hope inputmode is given a chance, and ends up being added to HTML5; I can't count the number of times I wish it was available on Web sites I visit from my mobile devices...
<rdar://problem/6234688>
Is the issue trying to be tackled here resolved by the new HTML5 input types like "tel" and "number" ? There's only a few things that seem to be missing wrt. to inputmode like telling the browser not to capitalize the first letter.
Yesterday Hixie added a new inputmode attribute to the current HTML5 spec: http://www.whatwg.org/specs/web-apps/current-work/multipage/association-of-controls-and-forms.html#attr-inputmode It's not based on the existing spec for the XHTML Basic 1.1 inputmode attribute. It doesn't (yet) allow modifier tokens, for one thing.