In the case of Windows 8, text input state including on-screen keyboard is controlled by the value of WebTextInputType returned from WebViewImpl::textInputType(). In past, it returns WebTextInputTypeDate for date text input but now it returns WebTextInputTypeNone. WebTextInputTypeNone is used for non editable node, so on-screen keyboard will be hidden if the date text input is focused. So in the case of Windows 8 tablet without any physical keyboards, tapping up/down arrow is the only way to input date.
Created attachment 190022 [details] Patch
I think this looks good. CCing morrita@ for review and tkent@ as FYI.
Comment on attachment 190022 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=190022&action=review > Source/WebKit/chromium/ChangeLog:7 > + because textInputType returns same value as non editable element. Please put information in https://bugs.webkit.org/show_bug.cgi?id=110740#c0 here. > Source/WebKit/chromium/ChangeLog:10 > + (WebKit::WebViewImpl::textInputInfo): Please write why this change is necessary. > Source/WebKit/chromium/src/WebViewImpl.cpp:2391 > + if (node->isElementNode()) { > + HTMLElement* element = static_cast<HTMLElement*>(node); This can cause bad casts. You need to check isHTMLElement. > Source/WebKit/chromium/src/WebViewImpl.cpp:2393 > + if (element->isDateTimeFieldElement()) > + return WebTextInputTypeDate; Using WebTextInputTypeDate looks wrong. - We're going to remove it as written in WebTextInputType.h. - This issue affects all of date/time types. WebTextInputTypeDate is only for input[type=date]. I think using WebTextInputTypeText is better.
Comment on attachment 190022 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=190022&action=review >> Source/WebKit/chromium/src/WebViewImpl.cpp:2393 >> + return WebTextInputTypeDate; > > Using WebTextInputTypeDate looks wrong. > - We're going to remove it as written in WebTextInputType.h. > - This issue affects all of date/time types. WebTextInputTypeDate is only for input[type=date]. > > I think using WebTextInputTypeText is better. The best solution is to introduce WebTextInputTypeDateTimeField.
Created attachment 190189 [details] Patch
Comment on attachment 190022 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=190022&action=review >> Source/WebKit/chromium/ChangeLog:7 >> + because textInputType returns same value as non editable element. > > Please put information in https://bugs.webkit.org/show_bug.cgi?id=110740#c0 here. Done. >> Source/WebKit/chromium/ChangeLog:10 >> + (WebKit::WebViewImpl::textInputInfo): > > Please write why this change is necessary. Done. >> Source/WebKit/chromium/src/WebViewImpl.cpp:2391 >> + HTMLElement* element = static_cast<HTMLElement*>(node); > > This can cause bad casts. You need to check isHTMLElement. Done. >>> Source/WebKit/chromium/src/WebViewImpl.cpp:2393 >>> + return WebTextInputTypeDate; >> >> Using WebTextInputTypeDate looks wrong. >> - We're going to remove it as written in WebTextInputType.h. >> - This issue affects all of date/time types. WebTextInputTypeDate is only for input[type=date]. >> >> I think using WebTextInputTypeText is better. > > The best solution is to introduce WebTextInputTypeDateTimeField. We can't use WebTextInputTypeText for other reasons, so let me introduce WebTextInputTypeDateTimeField.
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.
Comment on attachment 190189 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=190189&action=review > Source/WebKit/chromium/ChangeLog:18 > + With this patch, textInputInfo fills type filed regardless of editable or not, but it is > + safe because textInputType returns editable type only for known editable element. > + > + * src/WebViewImpl.cpp: > + (WebKit::WebViewImpl::textInputInfo): The paragraph should be on the function list like: * src/WebViewImpl.cpp: (WebKit::WebViewImpl::textInputInfo): Fills type filed regardless of editable or not. It is safe because textInputType returns editable type only for known editable element. > Source/WebKit/chromium/src/WebViewImpl.cpp:2391 > + if (node->isElementNode()) { > + HTMLElement* element = static_cast<HTMLElement*>(node); The code should be: if (node->isHTMLElement()) { HTMLElement* element = toHTMLElement(node); or if (node->isElementNode()) { Element* element = toElement(node);
Created attachment 190446 [details] Patch
Comment on attachment 190189 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=190189&action=review >> Source/WebKit/chromium/ChangeLog:18 >> + (WebKit::WebViewImpl::textInputInfo): > > The paragraph should be on the function list like: > > * src/WebViewImpl.cpp: > (WebKit::WebViewImpl::textInputInfo): Fills type filed regardless of editable or not. It is safe because textInputType returns editable type only for known editable element. Sure, sorry for my misunderstanding. >> Source/WebKit/chromium/src/WebViewImpl.cpp:2391 >> + HTMLElement* element = static_cast<HTMLElement*>(node); > > The code should be: > > if (node->isHTMLElement()) { > HTMLElement* element = toHTMLElement(node); > > or > > if (node->isElementNode()) { > Element* element = toElement(node); Sure, done.
Comment on attachment 190446 [details] Patch ok
Comment on attachment 190446 [details] Patch Clearing flags on attachment: 190446 Committed r144179: <http://trac.webkit.org/changeset/144179>
All reviewed patches have been landed. Closing bug.
Re-opened since this is blocked by bug 110980
This patch broke build: obj/content/renderer/content_renderer.render_widget.o ../../content/renderer/render_widget.cc:1978:error: size of array 'mismatching_enum' is negative ../../content/renderer/render_widget.cc:1980:error: size of array 'mismatching_enum' is negative ../../content/renderer/render_widget.cc:1982:error: size of array 'mismatching_enum' is negative ../../content/renderer/render_widget.cc:1984:error: size of array 'mismatching_enum' is negative ../../content/renderer/render_widget.cc:1986:error: size of array 'mismatching_enum' is negative ../../content/renderer/render_widget.cc:1988:error: size of array 'mismatching_enum' is negative ../../content/renderer/render_widget.cc:1990:error: size of array 'mismatching_enums' is negative ../../content/renderer/render_widget.cc:1990:error: conflicting declaration 'typedef struct GpuCompileAssert<false> content::mismatching_enums [1]' ../../content/renderer/render_widget.cc:1976:error: 'content::mismatching_enums' has a previous declaration as 'typedef struct GpuCompileAssert<true> content::mismatching_enums [1]' ../../content/renderer/render_widget.cc:1992:error: size of array 'mismatching_enums' is negative ../../content/renderer/render_widget.cc:1992:error: conflicting declaration 'typedef struct GpuCompileAssert<false> content::mismatching_enums [1]' ../../content/renderer/render_widget.cc:1976:error: 'content::mismatching_enums' has a previous declaration as 'typedef struct GpuCompileAssert<true> content::mismatching_enums [1]' ninja: build stopped: subcommand failed.
Reverted: http://trac.webkit.org/changeset/144185
Sorry for build break. There is a compile assert for checking enum compatibility. To fix the build breakage, we should add new enum entry TEXT_INPUT_TYPE_DATE_TIME_FIELD into ui/base/ime/text_input_type.h but it should be done at the same time of landing this patch. Otherwise build will be break again. I will remove compilation assertion temporary. Then after landing this patch, I will add compilation assertion again with new enum sets. Thank you.
James suggested me that we should add new enum entry into the end to preserve existing value. So I will update the patch. Thank you.
Created attachment 190538 [details] Patch
Comment on attachment 190538 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=190538&action=review > Source/WebKit/chromium/public/WebTextInputType.h:64 > // Input caret is in a contenteditable node (not an INPUT field). > WebTextInputTypeContentEditable, > + WebTextInputTypeDateTimeField, WebTextInputTypeDateTimeField looks like a kind of contenteditable because of the comment.
Created attachment 190642 [details] Patch
Comment on attachment 190538 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=190538&action=review >> Source/WebKit/chromium/public/WebTextInputType.h:64 >> + WebTextInputTypeDateTimeField, > > WebTextInputTypeDateTimeField looks like a kind of contenteditable because of the comment. Sure, added comment for date time field.
Comment on attachment 190642 [details] Patch ok
Comment on attachment 190642 [details] Patch Clearing flags on attachment: 190642 Committed r144261: <http://trac.webkit.org/changeset/144261>