| Summary: | Click events only fire once when editing | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Aditya Keerthi <pxlcoder> | ||||||
| Component: | HTML Editing | Assignee: | Nobody <webkit-unassigned> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | bdakin, commit-queue, megan_gardner, pxlcoder, thorton, webkit-bug-importer, wenson_hsieh | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | iPhone / iPad | ||||||||
| OS: | iOS 11 | ||||||||
| Attachments: |
|
||||||||
|
Description
Aditya Keerthi
2018-05-18 11:55:19 PDT
Created attachment 340726 [details]
Patch
Comment on attachment 340726 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=340726&action=review > Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:714 > + if (node->document().focusedElement() == node) We probably only want to bail here if we're also editing... Otherwise, this means that if an element that's not in editable content already has focus, we will no longer flash the tap highlight over that node. For instance, if you have a <button id='button'> and call button.focus(), we should still see the tap highlight over the button if you tap on it. Perhaps we could check if m_assistedNode == node instead? Comment on attachment 340726 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=340726&action=review >> Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:714 >> + if (node->document().focusedElement() == node) > > We probably only want to bail here if we're also editing... > > Otherwise, this means that if an element that's not in editable content already has focus, we will no longer flash the tap highlight over that node. > For instance, if you have a <button id='button'> and call button.focus(), we should still see the tap highlight over the button if you tap on it. Perhaps we could check if m_assistedNode == node instead? Actually, checking against m_assistedNode is probably still won't work in the case of editable elements that have been programmatically focused (select, input, textarea, etc.) prior to the tap... We really only want to suppress the tap highlight if we've tapped inside a node that is assisted in the UI process (i.e. the keyboard is up and the selection assistant is active). We also have m_isAssistingNodeDueToUserInteraction in the WebProcess. I'm going to see if I can use that. Created attachment 340738 [details]
Patch
Wenson pointed out that we can't use m_isAssistingNodeDueToUserInteraction in order to bail out of highlighting, due to a corner case. Clients that implement _webView:focusShouldStartInputSession: can programmatically focus editable elements and show the keyboard. This would result in m_isAssistingNodeDueToUserInteraction remaining false, and a tap highlight showing if a user were to tap on the element while editing. For this reason, I have moved the tap highlight check to the UIProcess. Comment on attachment 340738 [details]
Patch
Megan and I looked at this together.
Comment on attachment 340738 [details] Patch Clearing flags on attachment: 340738 Committed r232040: <https://trac.webkit.org/changeset/232040> All reviewed patches have been landed. Closing bug. |