Bug 214434 – should be able to send keyboard events to `<input readonly>` with a software keyboard

• Component: HTML Editing
• Status: NEW

[Devin Rousso]

`readonly` only means that the `value` is *not* modified when attempting to the edit the `<input>`.  `KeyboardEvent` can still be dispatched from an `<input readonly>`, and it can also be focusable.

[Devin Rousso]

<rdar://problem/65017950>

[Devin Rousso]

Created attachment 404499 [details]
Patch

[Devin Rousso]

Created attachment 404507 [details]
Patch

[Daniel Bates]

Comment on attachment 404507 [details]
Patch

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

Quick glance...Is it a good user experience on iPhone (but to a lesser extent iPad and whatever else) to show the software keyboard for a readonly, even though typing will 99% of the time do nothing? Could this cause confusion? More bug reports?

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:-1938
> -    if (_focusedElementInformation.isReadOnly)

This is ok as-is. The optimal solution also:

1. Patches equivalent code in Legacy WebKit for consistency

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:-5211
> -        completionHandler(isFocused && isEditable ? strongSelf.get() : nil);

This is ok as-is. The optimal solution does:

1. patches -requesrTextInputContexts **code path** because:
    A. Now it is **asymmetric**: this allows read only, but -request does not.
2. Patches up -focus equivalents in legacy WebKit for consistency
3. Adds a unit test for this change..grep unit tests for existing files

[Daniel Bates]

Hi Devin,

After thinking about this bug some more I think it is best addressed as an evangelism issue because:
    1. shopmini is purposely making the text field readonly and registering a click event handler to make it writable.
    2. Because of (1) the field is **not** tab focusable on Mac and iOS: a click or tap is always required to actually
    type into the field.
        a. Actually this behavior effects every browser and platform though I didn't check if the same content is served.
    3. Because of (2) the page is less accessible to people with disabilites.
    4. Quick grep for "readonly" and variations found radars of I hope now fixed regressions from people that asked that
    WebKit should **not** show the keyboard for readonly controls.

Second (optional) thing to do is to add a quirk for shopmini. Evangelism and quirk are a better solutions because:
    A. (1) is such a non-standard thing to do
    B. There are years of precedent for adding site-specific quirks to address things like (1).

I talked with Morgan Winer about this bug and to look at things another way, the reasons that showing a keyboard for a readonly field are less optimal are because:
    I. It leads to a broken user experience on 99% of sites that use readonly: people would likely expect that the field is editable and either
        i. Not understand why the keyboard is up without a blinking caret.
        ii. Not understand why none of the keyboard inputs are doing anything.
    II. Because of (I) this can lead to more reported bugs.

What's the quirk?

Make -focusTextInputContext simulate what happens when a person clicks the field.

I am going to re-purpose all 1, 2, 3, A, B, etc etc below because I ran out of easily typed alphabets...haha.

The optimal solution does this like so:

    1. Add a new setting that is disabled by default (and that will only be enabled in layout tests or API tests) because:
        A. This will allow tests to be written to ensure the quirk works and does not regress.
    2. Add a quirk to Quirks.cpp that returns true if (1) OR if the document's URL's host is shopmini or subdomain of it.
        A. My strong opinion, weakly held is that the quirk can be applied to all documents, not just the top document, because doing so will fix this site even if framed.

To see an example of a patch that does ^^^ see the patch on bug #199587.

    3. Page::editableElementsInRect's rootEditableElement lambda needs to be patched to collect readonly form controls (not just <input>s) when setting is enabled. That's the optimal solution because:
        A. A tiny bit defensive in case this site uses similar behavior for <textarea>s.
        B. Future proofs this quirk should it turn out it needs to be applied to other web sites that might also apply behavior to <textarea>s.

    4. Patch RenderBlock::paintObject's EventRegion code to add the text control to the region if readonly and the setting is enabled. This is needed because:
        A. EventRegion is the perf optimization used to allow the UI process to quickly know anything is editable without having to message the web process
        to know nothing is hit.
        A. Because of (A) it avoids -requestTextInputContexts from bailing because it thinks there are no editable fields on the page.

To see an example for how to write a test for (4) see LayoutTests/editing/editable-region

    5. Patch WebPage::focusTextInputContextAndPlaceCaret so that when setting in (1) is enabled it dispatches an event named mousedown BEFORE target is focused and dispatch mouseup and click named events BEFORE computing the editable postion (the latter because A) the event handler could adjust the element bounds and move it and B) it future proofs the code to more gracefully handle if (A) happens, which I am not going to talk about right now because this text is getting long).

To see an example of ^^^ see the patch on bug #209380. ^^^ can be tested using API tests. To see an example of this see the tests in RequestTextInputContext.mm grep for focusText

    6. Patch up -_elementDidFocus: to update focusedElementInformation.isReadOnly if the elementType and interactionRect are idenitical (grep for the FIXME: We should remove this check). Optimal solution calls -elementDidBlur if field BECOMES readonly to clear focused element state in ui process and most importantly hide the keyboard.
        A. Optimal solution adds tests just for ^^^^.
        B. Also can do ^^^ if field BECOMES disabled as well as I don't think web process sends -elementDidBlur...

    7. Add tests for (1) - (6).
    8. Patch up equivalent code in Legacy WebKit that does (5) and (6).