Don't flash a tap highlight for the entirety of an editable WKWebView
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?
https://trac.webkit.org/changeset/211343