Bug 193565 – iOS: Updating input mode should update the software keyboard

• Component: DOM
• Status: RESOLVED FIXED

[Ryosuke Niwa]

When inputmode content attribute is set on a focused assisted element,
we should update the software keyboard's type.

[Radar WebKit Bug Importer]

<rdar://problem/47376334>

[Ryosuke Niwa]

Created attachment 359452 [details]
WIP

[EWS Watchlist]

Attachment 359452 [details] did not pass style-queue:


ERROR: Source/WebCore/page/ChromeClient.h:297:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
Total errors found: 1 in 16 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Megan Gardner]

Comment on attachment 359452 [details]
WIP

Log files?
Not sure what you should do about ChromeClient.h now being compliant.
Everything else looks good.

[Wenson Hsieh]

Comment on attachment 359452 [details]
WIP

View in context: https://bugs.webkit.org/attachment.cgi?id=359452&action=review

This patch is only a WIP, as it wasn't marked for review and it's also still missing ChangeLog entries and tests.

That being said, the codechange itself also looks good to me.

> Source/WebCore/html/HTMLElement.cpp:457
> +            document.page()->chrome().client().focusedElementDidChangeInputMode(*this, canonicalInputMode());

Does something guarantee that page is non-null here?

> Source/WebKit/Scripts/webkit/messages.py:410
> +        'WebCore::InputMode': ['<WebCore/InputMode.h>'],

A little surprising that this is needed! I thought the code generator would (by default) try to include <WebCore/InputMode.h> based on the type of the IPC argument, but maybe this isn't the case for enum class types.

[Ryosuke Niwa]

Created attachment 359582 [details]
Fixes the bug

[EWS Watchlist]

Attachment 359582 [details] did not pass style-queue:


ERROR: Source/WebCore/page/ChromeClient.h:297:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
Total errors found: 1 in 21 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Wenson Hsieh]

Comment on attachment 359582 [details]
Fixes the bug

View in context: https://bugs.webkit.org/attachment.cgi?id=359582&action=review

> Source/WebKit/WebProcess/WebPage/WebPage.cpp:5382
> +    UNUSED_PARAM(element);

Extra UNUSED_PARAM(element); here.

[Ryosuke Niwa]

Created attachment 359583 [details]
Patch for landing

[Ryosuke Niwa]

Comment on attachment 359583 [details]
Patch for landing

Wait for EWS.

[EWS Watchlist]

Attachment 359583 [details] did not pass style-queue:


ERROR: Source/WebCore/page/ChromeClient.h:297:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
Total errors found: 1 in 21 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Ryosuke Niwa]

Comment on attachment 359583 [details]
Patch for landing

Clearing flags on attachment: 359583

Committed r240199: <https://trac.webkit.org/changeset/240199>

[Ryosuke Niwa]

All reviewed patches have been landed.  Closing bug.

[David Kilzer (:ddkilzer)]

(In reply to Ryosuke Niwa from comment #12)
> Comment on attachment 359583 [details]
> Patch for landing
> 
> Clearing flags on attachment: 359583
> 
> Committed r240199: <https://trac.webkit.org/changeset/240199>

This broken all iOS Debug builds:

/Volumes/Data/slave/ios-simulator-12-debug/build/Source/WebKit/WebProcess/WebPage/WebPage.cpp:5368:20: error: no member named 'canonicalInputMode' in 'WebCore::Element'
    ASSERT(element.canonicalInputMode() == mode);
           ~~~~~~~ ^

The `element` variable needs to be `downcast<HTMLElement>(element)` here, and should probably add an `is<HTMLElement>(element)` check as well.

Testing a fix now.

[David Kilzer (:ddkilzer)]

(In reply to David Kilzer (:ddkilzer) from comment #14)
> (In reply to Ryosuke Niwa from comment #12)
> > Comment on attachment 359583 [details]
> > Patch for landing
> > 
> > Clearing flags on attachment: 359583
> > 
> > Committed r240199: <https://trac.webkit.org/changeset/240199>
> 
> This broken all iOS Debug builds:
> 
> /Volumes/Data/slave/ios-simulator-12-debug/build/Source/WebKit/WebProcess/
> WebPage/WebPage.cpp:5368:20: error: no member named 'canonicalInputMode' in
> 'WebCore::Element'
>     ASSERT(element.canonicalInputMode() == mode);
>            ~~~~~~~ ^
> 
> The `element` variable needs to be `downcast<HTMLElement>(element)` here,
> and should probably add an `is<HTMLElement>(element)` check as well.
> 
> Testing a fix now.

Should be fixed by:

Committed r240203: <https://trac.webkit.org/changeset/240203>

[Wenson Hsieh]

Thanks, Dave!