| Summary: | Don't flash a tap highlight for the entirety of an editable WKWebView | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Tim Horton <thorton> | ||||||
| Component: | New Bugs | Assignee: | Tim Horton <thorton> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | cdumez, commit-queue, dbates, enrica, esprehn+autocc, kangil.han, mitz | ||||||
| Priority: | P2 | ||||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| See Also: | https://bugs.webkit.org/show_bug.cgi?id=167488 | ||||||||
| Attachments: |
|
||||||||
|
Description
Tim Horton
2017-01-26 20:35:35 PST
Created attachment 299903 [details]
Patch
Comment on attachment 299903 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=299903&action=review > Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:639 > + if (is<HTMLBodyElement>(node) && m_page->isEditable()) > + return; > + Does this exclude the body of subframes as well? Is that expected? That is a fair question; this will certainly stop flashing in that case too, but I don't know that that is necessarily a bad thing. To me, the tap highlight is much more about small buttons/links/input fields, and less about (usually) large things like frames. But we could limit it to just the main frame body. What do you think? (In reply to comment #3) > That is a fair question; this will certainly stop flashing in that case too, > but I don't know that that is necessarily a bad thing. To me, the tap > highlight is much more about small buttons/links/input fields, and less > about (usually) large things like frames. But we could limit it to just the > main frame body. What do you think? If the goal is to make editable views behave more like text views, or to “not flash a tap highlight for the entirety of an editable WKWebView”, then applying this to subframes’ bodies doesn’t seem to support the goal. I believe that there is already a size-based filter to prevent large highlights, so if a subframe’s body or an image are sufficiently large, they won’t highlight. There is no reason to infer size from element type. (In reply to comment #4) > (In reply to comment #3) > > That is a fair question; this will certainly stop flashing in that case too, > > but I don't know that that is necessarily a bad thing. To me, the tap > > highlight is much more about small buttons/links/input fields, and less > > about (usually) large things like frames. But we could limit it to just the > > main frame body. What do you think? > > If the goal is to make editable views behave more like text views, I think that is the goal. > or to “not flash a tap highlight for the entirety of an editable WKWebView”, then > applying this to subframes’ bodies doesn’t seem to support the goal. I > believe that there is already a size-based filter to prevent large > highlights, Does not seem like a very good filter if it lets a iPad-sized <body> get highlighted, but OK. > so if a subframe’s body or an image are sufficiently large, they > won’t highlight. There is no reason to infer size from element type. Fair! I will adjust the patch. (In reply to comment #5) > Does not seem like a very good filter if it lets a iPad-sized <body> get > highlighted, but OK. Filed bug 167488 for that. Created attachment 299982 [details]
Patch
Comment on attachment 299982 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=299982&action=review > Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:637 > + if (node == m_page->mainFrame().document()->body() && m_page->isEditable()) Is it better to evaluate these in the reverse order? |