Changeset 230527 in webkit

Timestamp:

Apr 11, 2018 9:43:53 AM (6 years ago)

Author:

Wenson Hsieh

Message:

[Extra zoom mode] Missing label when focusing a <select> with a title attribute but no associated <label>
​https://bugs.webkit.org/show_bug.cgi?id=184352
<rdar://problem/39237683>

Reviewed by Andy Estes.

Source/WebKit:

Currently, AssistedNodeInformation only sends the title of input elements to the UI process. This means that
any information requested in the UI process that is dependent on the title of the focused element is broken
in the case of select elements. An existing example of this is the title of the table view controller used to
present select menus on iPad.

To fix this, we simply send the title of the focused element across, as long as the focused element is an
HTMLElement. This ensures that there's label text when focusing unlabeled select elements with titles in extra
zoom mode, and also fixes a currenly broken codepath where we show the title of the select in the presented view
controller's title.

Test: fast/forms/ios/ipad/select-with-title.html

• UIProcess/API/Cocoa/WKWebView.mm:

(-[WKWebView selectFormPopoverTitle]):

• UIProcess/API/Cocoa/WKWebViewPrivate.h:

Add new testing SPI to fetch the title of the UITableViewController presented for the currently focused select
element.

• UIProcess/ios/WKContentViewInteraction.h:
• UIProcess/ios/WKContentViewInteraction.mm:

(-[WKContentView selectFormPopoverTitle]):

• UIProcess/ios/forms/WKFormSelectControl.h:
• UIProcess/ios/forms/WKFormSelectControl.mm:

(-[WKFormSelectControl selectFormPopoverTitle]):

• UIProcess/ios/forms/WKFormSelectPopover.h:
• UIProcess/ios/forms/WKFormSelectPopover.mm:

(-[WKSelectPopover initWithView:hasGroups:]):
(-[WKSelectPopover tableViewController]):

• WebProcess/WebPage/ios/WebPageIOS.mm:

Always send the title across if the focused node is an HTMLElement.

(WebKit::WebPage::getAssistedNodeInformation):

Tools:

Add support for UIScriptController::selectFormPopoverTitle, which returns the title of the current select
popover's UITableViewController (for testing purposes).

• DumpRenderTree/ios/UIScriptControllerIOS.mm:

(WTR::UIScriptController::selectFormPopoverTitle const):

• TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl:
• TestRunnerShared/UIScriptContext/UIScriptController.cpp:

(WTR::UIScriptController::selectFormPopoverTitle const):

• TestRunnerShared/UIScriptContext/UIScriptController.h:
• WebKitTestRunner/ios/UIScriptControllerIOS.mm:

(WTR::UIScriptController::selectFormPopoverTitle const):

LayoutTests:

Add a new layout test to check that the title of select elements is propagated to the UI process upon focus.
This title is used for several purposes, one of them being the title of select menus in the table view
controller presented when tapping on a select on an iPad, so the test checks that the title of the select is
shown here.

Also moves a select-related helper into UIHelper from basic-gestures.js (since this doesn't involve user gesture
simulation in any way) and also introduces a new UIHelper method for querying the title of the select menu that
is currently being presented.

• fast/forms/ios/ipad/multiple-select-updates-renderer.html:
• fast/forms/ios/ipad/select-with-title-expected.txt: Added.
• fast/forms/ios/ipad/select-with-title.html: Copied from LayoutTests/fast/forms/ios/ipad/multiple-select-updates-renderer.html.
• resources/basic-gestures.js:
• resources/ui-helper.js:

(window.UIHelper.selectFormAccessoryPickerRow):
(window.UIHelper.selectFormPopoverTitle):
(window.UIHelper):

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/forms/ios/ipad/multiple-select-updates-renderer.html (modified) (2 diffs)
• LayoutTests/fast/forms/ios/ipad/select-with-title-expected.txt (added)
• LayoutTests/fast/forms/ios/ipad/select-with-title.html (copied) (copied from trunk/LayoutTests/fast/forms/ios/ipad/multiple-select-updates-renderer.html) (3 diffs)
• LayoutTests/resources/basic-gestures.js (modified) (1 diff)
• LayoutTests/resources/ui-helper.js (modified) (1 diff)
• Source/WebKit/ChangeLog (modified) (1 diff)
• Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm (modified) (1 diff)
• Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h (modified) (1 diff)
• Source/WebKit/UIProcess/ios/WKContentViewInteraction.h (modified) (1 diff)
• Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm (modified) (1 diff)
• Source/WebKit/UIProcess/ios/forms/WKFormSelectControl.h (modified) (1 diff)
• Source/WebKit/UIProcess/ios/forms/WKFormSelectControl.mm (modified) (1 diff)
• Source/WebKit/UIProcess/ios/forms/WKFormSelectPopover.h (modified) (2 diffs)
• Source/WebKit/UIProcess/ios/forms/WKFormSelectPopover.mm (modified) (2 diffs)
• Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm (modified) (2 diffs)
• Tools/ChangeLog (modified) (1 diff)
• Tools/DumpRenderTree/ios/UIScriptControllerIOS.mm (modified) (1 diff)
• Tools/TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl (modified) (1 diff)
• Tools/TestRunnerShared/UIScriptContext/UIScriptController.cpp (modified) (1 diff)
• Tools/TestRunnerShared/UIScriptContext/UIScriptController.h (modified) (1 diff)
• Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2018-04-11  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [Extra zoom mode] Missing label when focusing a <select> with a title attribute but no associated <label>
+        https://bugs.webkit.org/show_bug.cgi?id=184352
+        <rdar://problem/39237683>
+
+        Reviewed by Andy Estes.
+
+        Add a new layout test to check that the title of select elements is propagated to the UI process upon focus.
+        This title is used for several purposes, one of them being the title of select menus in the table view
+        controller presented when tapping on a select on an iPad, so the test checks that the title of the select is
+        shown here.
+
+        Also moves a select-related helper into UIHelper from basic-gestures.js (since this doesn't involve user gesture
+        simulation in any way) and also introduces a new UIHelper method for querying the title of the select menu that
+        is currently being presented.
+
+        * fast/forms/ios/ipad/multiple-select-updates-renderer.html:
+        * fast/forms/ios/ipad/select-with-title-expected.txt: Added.
+        * fast/forms/ios/ipad/select-with-title.html: Copied from LayoutTests/fast/forms/ios/ipad/multiple-select-updates-renderer.html.
+        * resources/basic-gestures.js:
+        * resources/ui-helper.js:
+        (window.UIHelper.selectFormAccessoryPickerRow):
+        (window.UIHelper.selectFormPopoverTitle):
+        (window.UIHelper):
+
 2018-04-10  Ryan Haddad  <ryanhaddad@apple.com>
 

trunk/LayoutTests/fast/forms/ios/ipad/multiple-select-updates-renderer.html

@@ -15 +15 @@
 </style>
 <script src="../../../../resources/basic-gestures.js"></script>
+<script src="../../../../resources/ui-helper.js"></script>
 <script>
     async function run()
@@ -22 +23 @@
 
         await tapAtPoint(100, 100);
-        await selectFormAccessoryPickerRow(0);
+        await UIHelper.selectFormAccessoryPickerRow(0);
         doneEvaluatingUIScript = true;
         checkDone();

trunk/LayoutTests/fast/forms/ios/ipad/select-with-title.html

@@ -14 +14 @@
     }
 </style>
+<script src="../../../../resources/js-test.js"></script>
+<script src="../../../../resources/ui-helper.js"></script>
 <script src="../../../../resources/basic-gestures.js"></script>
 <script>
@@ -22 +24 @@
 
         await tapAtPoint(100, 100);
-        await selectFormAccessoryPickerRow(0);
-        doneEvaluatingUIScript = true;
+        popoverTitle = await UIHelper.selectFormPopoverTitle();
+        await UIHelper.selectFormAccessoryPickerRow(1);
         checkDone();
     }
@@ -29 +31 @@
     function checkDone()
     {
-        if (window.doneEvaluatingUIScript && window.valueChanged && window.testRunner)
+        doneCount = 1 + (window.doneCount ? doneCount : 0);
+        if (doneCount == 2 && window.testRunner) {
+            shouldBe("popoverTitle", "document.getElementById('select').title");
             testRunner.notifyDone();
+        }
     }
 </script>
 </head>
 <body onload="run()">
-    <select multiple id="select"><option>This is an option.</option></select>
+    <select title="This is a title" id="select">
+        <option>1</option>
+        <option>2</option>
+        <option>3</option>
+    </select>
     <pre id="output"></pre>
 </body>
 <script>
-    select.addEventListener("change", () => {
-        if (window.internals) {
-            const renderTreeAsText = internals.elementRenderTreeAsText(document.documentElement);
-            const expectedOptionValue = "This is an option.";
-            if (renderTreeAsText.indexOf(expectedOptionValue) != -1)
-                output.textContent = "PASS";
-            else
-                output.textContent = `FAIL: expected '${expectedOptionValue}' in render tree dump:\n${renderTreeAsText}`;
-        }
-
-        select.blur();
-        valueChanged = true;
-        checkDone();
-    });
+    select.addEventListener("blur", checkDone);
+    select.addEventListener("change", () => select.blur());
 
     if (window.testRunner) {

trunk/LayoutTests/resources/basic-gestures.js

@@ -139 +139 @@
             })();`, resolve);
     });
-}
-
-function selectFormAccessoryPickerRow(rowIndex)
-{
-    const selectRowScript = `(() => uiController.selectFormAccessoryPickerRow(${rowIndex}))()`;
-    return new Promise(resolve => testRunner.runUIScript(selectRowScript, resolve));
 }
 

trunk/LayoutTests/resources/ui-helper.js

@@ -162 +162 @@
         internals.withUserGesture(callback);
     }
+
+    static selectFormAccessoryPickerRow(rowIndex)
+    {
+        const selectRowScript = `(() => uiController.selectFormAccessoryPickerRow(${rowIndex}))()`;
+        return new Promise(resolve => testRunner.runUIScript(selectRowScript, resolve));
+    }
+
+    static selectFormPopoverTitle()
+    {
+        return new Promise(resolve => {
+            testRunner.runUIScript(`(() => {
+                uiController.uiScriptComplete(uiController.selectFormPopoverTitle);
+            })()`, resolve);
+        });
+    }
 }

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2018-04-11  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [Extra zoom mode] Missing label when focusing a <select> with a title attribute but no associated <label>
+        https://bugs.webkit.org/show_bug.cgi?id=184352
+        <rdar://problem/39237683>
+
+        Reviewed by Andy Estes.
+
+        Currently, AssistedNodeInformation only sends the `title` of input elements to the UI process. This means that
+        any information requested in the UI process that is dependent on the `title` of the focused element is broken
+        in the case of select elements. An existing example of this is the title of the table view controller used to
+        present select menus on iPad.
+
+        To fix this, we simply send the `title` of the focused element across, as long as the focused element is an
+        HTMLElement. This ensures that there's label text when focusing unlabeled select elements with titles in extra
+        zoom mode, and also fixes a currenly broken codepath where we show the title of the select in the presented view
+        controller's title.
+
+        Test: fast/forms/ios/ipad/select-with-title.html
+
+        * UIProcess/API/Cocoa/WKWebView.mm:
+        (-[WKWebView selectFormPopoverTitle]):
+        * UIProcess/API/Cocoa/WKWebViewPrivate.h:
+
+        Add new testing SPI to fetch the title of the UITableViewController presented for the currently focused select
+        element.
+
+        * UIProcess/ios/WKContentViewInteraction.h:
+        * UIProcess/ios/WKContentViewInteraction.mm:
+        (-[WKContentView selectFormPopoverTitle]):
+        * UIProcess/ios/forms/WKFormSelectControl.h:
+        * UIProcess/ios/forms/WKFormSelectControl.mm:
+        (-[WKFormSelectControl selectFormPopoverTitle]):
+        * UIProcess/ios/forms/WKFormSelectPopover.h:
+        * UIProcess/ios/forms/WKFormSelectPopover.mm:
+        (-[WKSelectPopover initWithView:hasGroups:]):
+        (-[WKSelectPopover tableViewController]):
+        * WebProcess/WebPage/ios/WebPageIOS.mm:
+
+        Always send the title across if the focused node is an HTMLElement.
+
+        (WebKit::WebPage::getAssistedNodeInformation):
+
 2018-04-11  Ryan Haddad  <ryanhaddad@apple.com>
 

trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm

@@ -5872 +5872 @@
 }
 
+- (NSString *)selectFormPopoverTitle
+{
+    return [_contentView selectFormPopoverTitle];
+}
+
 - (void)didStartFormControlInteraction
 {

trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h

@@ -382 +382 @@
 - (void)dismissFormAccessoryView WK_API_AVAILABLE(ios(10.3));
 - (void)selectFormAccessoryPickerRow:(int)rowIndex WK_API_AVAILABLE(ios(10.3));
+@property (nonatomic, readonly) NSString *selectFormPopoverTitle WK_API_AVAILABLE(ios(WK_IOS_TBA));
 
 - (void)applyAutocorrection:(NSString *)newString toString:(NSString *)oldString withCompletionHandler:(void (^)(void))completionHandler WK_API_AVAILABLE(ios(11.0));

trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h

@@ -364 +364 @@
 - (NSDictionary *)_contentsOfUserInterfaceItem:(NSString *)userInterfaceItem;
 
+@property (nonatomic, readonly) NSString *selectFormPopoverTitle;
+
 @end
 

trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm

@@ -5440 +5440 @@
 }
 
+- (NSString *)selectFormPopoverTitle
+{
+    if (![_inputPeripheral isKindOfClass:[WKFormSelectControl self]])
+        return nil;
+
+    return [(WKFormSelectControl *)_inputPeripheral selectFormPopoverTitle];
+}
+
 - (NSDictionary *)_contentsOfUserInterfaceItem:(NSString *)userInterfaceItem
 {

trunk/Source/WebKit/UIProcess/ios/forms/WKFormSelectControl.h

@@ -42 +42 @@
 @interface WKFormSelectControl(WKTesting)
 - (void)selectRow:(NSInteger)rowIndex inComponent:(NSInteger)componentIndex extendingSelection:(BOOL)extendingSelection;
+@property (nonatomic, readonly) NSString *selectFormPopoverTitle;
 @end
 

trunk/Source/WebKit/UIProcess/ios/forms/WKFormSelectControl.mm

@@ -112 +112 @@
 }
 
+- (NSString *)selectFormPopoverTitle
+{
+    if (![_control isKindOfClass:[WKSelectPopover class]])
+        return nil;
+
+    return [(WKSelectPopover *)_control.get() tableViewController].title;
+}
+
 @end
 

trunk/Source/WebKit/UIProcess/ios/forms/WKFormSelectPopover.h

@@ -29 +29 @@
 #import "WKFormPopover.h"
 
+@class UITableViewController;
 @class WKContentView;
 
@@ -34 +35 @@
 - (instancetype)initWithView:(WKContentView *)view hasGroups:(BOOL)hasGroups;
 - (void)_userActionDismissedPopover:(id)sender;
+@property (nonatomic, readonly) UITableViewController *tableViewController;
 @end
 

trunk/Source/WebKit/UIProcess/ios/forms/WKFormSelectPopover.mm

@@ -390 +390 @@
     UIViewController *popoverViewController = _tableViewController.get();
     UINavigationController *navController = nil;
-    NSString *title = view.assistedNodeInformation.title;
-    BOOL needsNavigationController = !!title.length;
+    BOOL needsNavigationController = !view.assistedNodeInformation.title.isEmpty();
     if (needsNavigationController) {
         navController = [[UINavigationController alloc] initWithRootViewController:_tableViewController.get()];
@@ -443 +442 @@
 }
 
+- (UITableViewController *)tableViewController
+{
+    return _tableViewController.get();
+}
+
 @end
 

trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm

@@ -2787 +2787 @@
     }
 
+    if (is<HTMLElement>(m_assistedNode.get()))
+        information.title = downcast<HTMLElement>(*m_assistedNode).title();
+
     if (is<HTMLSelectElement>(*m_assistedNode)) {
         HTMLSelectElement& element = downcast<HTMLSelectElement>(*m_assistedNode);
@@ -2867 +2870 @@
         information.value = element.value();
         information.valueAsNumber = element.valueAsNumber();
-        information.title = element.title();
         information.autofillFieldName = WebCore::toAutofillFieldName(element.autofillData().fieldName);
     } else if (m_assistedNode->hasEditableStyle()) {

trunk/Tools/ChangeLog

@@ +1 @@
+2018-04-11  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [Extra zoom mode] Missing label when focusing a <select> with a title attribute but no associated <label>
+        https://bugs.webkit.org/show_bug.cgi?id=184352
+        <rdar://problem/39237683>
+
+        Reviewed by Andy Estes.
+
+        Add support for UIScriptController::selectFormPopoverTitle, which returns the title of the current select
+        popover's UITableViewController (for testing purposes).
+
+        * DumpRenderTree/ios/UIScriptControllerIOS.mm:
+        (WTR::UIScriptController::selectFormPopoverTitle const):
+        * TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl:
+        * TestRunnerShared/UIScriptContext/UIScriptController.cpp:
+        (WTR::UIScriptController::selectFormPopoverTitle const):
+        * TestRunnerShared/UIScriptContext/UIScriptController.h:
+        * WebKitTestRunner/ios/UIScriptControllerIOS.mm:
+        (WTR::UIScriptController::selectFormPopoverTitle const):
+
 2018-04-10  Nan Wang  <n_wang@apple.com>
 

trunk/Tools/DumpRenderTree/ios/UIScriptControllerIOS.mm

@@ -159 +159 @@
 {
 }
+
+JSRetainPtr<JSStringRef> UIScriptController::selectFormPopoverTitle() const
+{
+    return nullptr;
+}
     
 JSObjectRef UIScriptController::contentsOfUserInterfaceItem(JSStringRef interfaceItem) const

trunk/Tools/TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl

@@ -188 +188 @@
     // <select> picker
     void selectFormAccessoryPickerRow(long rowIndex);
+    readonly attribute DOMString selectFormPopoverTitle;
 
     void keyboardAccessoryBarNext();

trunk/Tools/TestRunnerShared/UIScriptContext/UIScriptController.cpp

@@ -299 +299 @@
 }
 
+JSRetainPtr<JSStringRef> UIScriptController::selectFormPopoverTitle() const
+{
+    return nullptr;
+}
+
 void UIScriptController::scrollToOffset(long x, long y)
 {

trunk/Tools/TestRunnerShared/UIScriptContext/UIScriptController.h

@@ -98 +98 @@
     void dismissFormAccessoryView();
     void selectFormAccessoryPickerRow(long);
+    JSRetainPtr<JSStringRef> selectFormPopoverTitle() const;
     
     JSObjectRef contentsOfUserInterfaceItem(JSStringRef) const;

trunk/Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm

@@ -419 +419 @@
 }
 
+JSRetainPtr<JSStringRef> UIScriptController::selectFormPopoverTitle() const
+{
+    TestRunnerWKWebView *webView = TestController::singleton().mainWebView()->platformView();
+    return JSStringCreateWithCFString((CFStringRef)webView.selectFormPopoverTitle);
+}
+
 void UIScriptController::selectFormAccessoryPickerRow(long rowIndex)
 {