144603 2015-05-04 15:57:26 -0700 [iOS WK2] editorState should include information about selection clipping rectangle 2015-05-04 16:52:57 -0700 1 1 1 Unclassified WebKit WebKit2 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 enrica enrica cdumez oldest_to_newest 1091569 0 enrica 2015-05-04 15:57:26 -0700 We currently provide the element rect only when the element is first focused and we use that to clip the selection before drawing. If the element is resized/repositioned via Javascript or CSS, we continue to use the clipping rectangle at the time the element was focused. Pages like google.com or imdb.com show the incorrect clipping of the caret. rdar://problem/20521252 1091574 1 252345 enrica 2015-05-04 16:03:05 -0700 Created attachment 252345 Patch 1091578 2 cdumez 2015-05-04 16:05:18 -0700 Did you try a debug build or turning the assertions in postLayoutData() into RELEASE_ASSERTs? 1091591 3 252345 cdumez 2015-05-04 16:23:34 -0700 Comment on attachment 252345 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=252345&action=review r=me with nits. > Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:190 > + Frame& elementFrame = m_page->focusController().focusedOrMainFrame(); I believe this is equivalent to the frame argument of this method. It is initialized in WebPage::editorState() as: Frame& frame = m_page->focusController().focusedOrMainFrame(); So this variable is not needed. > Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:191 > + postLayoutData.selectionClipRect = elementFrame.view()->contentsToRootView(m_assistedNode->renderer()->absoluteBoundingBoxRect()); postLayoutData.selectionClipRect = view->contentsToRootView(m_assistedNode->renderer()->absoluteBoundingBoxRect()); We already have a variable of the view. 1091609 4 enrica 2015-05-04 16:50:40 -0700 (In reply to comment #3) > Comment on attachment 252345 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=252345&action=review > > r=me with nits. > > > Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:190 > > + Frame& elementFrame = m_page->focusController().focusedOrMainFrame(); > > I believe this is equivalent to the frame argument of this method. It is > initialized in WebPage::editorState() as: > Frame& frame = m_page->focusController().focusedOrMainFrame(); > > So this variable is not needed. > > > Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:191 > > + postLayoutData.selectionClipRect = elementFrame.view()->contentsToRootView(m_assistedNode->renderer()->absoluteBoundingBoxRect()); > > postLayoutData.selectionClipRect = > view->contentsToRootView(m_assistedNode->renderer()- > >absoluteBoundingBoxRect()); > > We already have a variable of the view. Duh! This is what you get for copy/paste. Thanks for the review. I've fixed it. 1091610 5 enrica 2015-05-04 16:52:57 -0700 Committed revision 183780. 252345 2015-05-04 16:03:05 -0700 2015-05-04 16:23:34 -0700 Patch cliprectineditorstate.txt text/plain 4366 enrica SW5kZXg6IFNvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi S2l0Mi9DaGFuZ2VMb2cJKHJldmlzaW9uIDE4Mzc3NCkKKysrIFNvdXJjZS9XZWJLaXQyL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDI1IEBACisyMDE1LTA1LTA0ICBFbnJpY2Eg Q2FzdWNjaSAgPGVucmljYUBhcHBsZS5jb20+CisKKyAgICAgICAgW2lPUyBXSzJdIGVkaXRvclN0 YXRlIHNob3VsZCBpbmNsdWRlIGluZm9ybWF0aW9uIGFib3V0IHNlbGVjdGlvbiBjbGlwcGluZyBy ZWN0YW5nbGUuCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9p ZD0xNDQ2MDMKKyAgICAgICAgcmRhcjovL3Byb2JsZW0vMjA1MjEyNTIKKworICAgICAgICBSZXZp ZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBBZGRpbmcgc2VsZWN0aW9uQ2xpcFJl Y3QgdG8gdGhlIHBvc3QtbGF5b3V0IHBvcnRpb24gb2YgRWRpdG9yU3RhdGUuCisgICAgICAgIFRo aXMgd2F5IHdlIGNhbiBjb21wdXRlIHRoZSBzZWxlY3Rpb24gY2xpcHBpbmcgcmVjdGFuZ2xlIGJh c2VkIG9uCisgICAgICAgIHVwLXRvLWRhdGUgaW5mb3JtYXRpb24gaW5zdGVhZCBvZiB1c2luZyB0 aGUgcmVjdGFuZ2xlIHByb3ZpZGVkIHdoZW4KKyAgICAgICAgdGhlbiBlbGVtZW50IHdhcyBmaXJz dCBmb2N1c2VkLgorCisgICAgICAgICogU2hhcmVkL0VkaXRvclN0YXRlLmNwcDoKKyAgICAgICAg KFdlYktpdDo6RWRpdG9yU3RhdGU6OlBvc3RMYXlvdXREYXRhOjplbmNvZGUpOgorICAgICAgICAo V2ViS2l0OjpFZGl0b3JTdGF0ZTo6UG9zdExheW91dERhdGE6OmRlY29kZSk6CisgICAgICAgICog U2hhcmVkL0VkaXRvclN0YXRlLmg6CisgICAgICAgICogVUlQcm9jZXNzL2lvcy9XS0NvbnRlbnRW aWV3SW50ZXJhY3Rpb24ubW06CisgICAgICAgICgtW1dLQ29udGVudFZpZXcgX3NlbGVjdGlvbkNs aXBSZWN0XSk6CisgICAgICAgICogV2ViUHJvY2Vzcy9XZWJQYWdlL2lvcy9XZWJQYWdlSU9TLm1t OgorICAgICAgICAoV2ViS2l0OjpXZWJQYWdlOjpwbGF0Zm9ybUVkaXRvclN0YXRlKToKKwogMjAx NS0wNS0wNCAgQW50dGkgS29pdmlzdG8gIDxhbnR0aUBhcHBsZS5jb20+CiAKICAgICAgICAgTmV0 d29yayBDYWNoZTogU3VwcG9ydCB0aW1lIGJhc2VkIGNhY2hlIGNsZWFyaW5nCkluZGV4OiBTb3Vy Y2UvV2ViS2l0Mi9TaGFyZWQvRWRpdG9yU3RhdGUuY3BwCj09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9X ZWJLaXQyL1NoYXJlZC9FZGl0b3JTdGF0ZS5jcHAJKHJldmlzaW9uIDE4Mzc2MykKKysrIFNvdXJj ZS9XZWJLaXQyL1NoYXJlZC9FZGl0b3JTdGF0ZS5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTExMSw2 ICsxMTEsNyBAQCBib29sIEVkaXRvclN0YXRlOjpkZWNvZGUoSVBDOjpBcmd1bWVudERlCiAjaWYg UExBVEZPUk0oSU9TKQogdm9pZCBFZGl0b3JTdGF0ZTo6UG9zdExheW91dERhdGE6OmVuY29kZShJ UEM6OkFyZ3VtZW50RW5jb2RlciYgZW5jb2RlcikgY29uc3QKIHsKKyAgICBlbmNvZGVyIDw8IHNl bGVjdGlvbkNsaXBSZWN0OwogICAgIGVuY29kZXIgPDwgc2VsZWN0aW9uUmVjdHM7CiAgICAgZW5j b2RlciA8PCBjYXJldFJlY3RBdFN0YXJ0OwogICAgIGVuY29kZXIgPDwgY2FyZXRSZWN0QXRFbmQ7 CkBAIC0xMjYsNiArMTI3LDggQEAgdm9pZCBFZGl0b3JTdGF0ZTo6UG9zdExheW91dERhdGE6OmVu Y29kZQogCiBib29sIEVkaXRvclN0YXRlOjpQb3N0TGF5b3V0RGF0YTo6ZGVjb2RlKElQQzo6QXJn dW1lbnREZWNvZGVyJiBkZWNvZGVyLCBQb3N0TGF5b3V0RGF0YSYgcmVzdWx0KQogeworICAgIGlm ICghZGVjb2Rlci5kZWNvZGUocmVzdWx0LnNlbGVjdGlvbkNsaXBSZWN0KSkKKyAgICAgICAgcmV0 dXJuIGZhbHNlOwogICAgIGlmICghZGVjb2Rlci5kZWNvZGUocmVzdWx0LnNlbGVjdGlvblJlY3Rz KSkKICAgICAgICAgcmV0dXJuIGZhbHNlOwogICAgIGlmICghZGVjb2Rlci5kZWNvZGUocmVzdWx0 LmNhcmV0UmVjdEF0U3RhcnQpKQpJbmRleDogU291cmNlL1dlYktpdDIvU2hhcmVkL0VkaXRvclN0 YXRlLmgKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PQotLS0gU291cmNlL1dlYktpdDIvU2hhcmVkL0VkaXRvclN0YXRlLmgJ KHJldmlzaW9uIDE4Mzc2MykKKysrIFNvdXJjZS9XZWJLaXQyL1NoYXJlZC9FZGl0b3JTdGF0ZS5o CSh3b3JraW5nIGNvcHkpCkBAIC02MSw2ICs2MSw3IEBAIHN0cnVjdCBFZGl0b3JTdGF0ZSB7CiAg ICAgU3RyaW5nIG1hcmtlZFRleHQ7CiAKICAgICBzdHJ1Y3QgUG9zdExheW91dERhdGEgeworICAg ICAgICBXZWJDb3JlOjpJbnRSZWN0IHNlbGVjdGlvbkNsaXBSZWN0OwogICAgICAgICBWZWN0b3I8 V2ViQ29yZTo6U2VsZWN0aW9uUmVjdD4gc2VsZWN0aW9uUmVjdHM7CiAgICAgICAgIFdlYkNvcmU6 OkludFJlY3QgY2FyZXRSZWN0QXRTdGFydDsKICAgICAgICAgV2ViQ29yZTo6SW50UmVjdCBjYXJl dFJlY3RBdEVuZDsKSW5kZXg6IFNvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9pb3MvV0tDb250ZW50 Vmlld0ludGVyYWN0aW9uLm1tCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJLaXQyL1VJUHJvY2Vz cy9pb3MvV0tDb250ZW50Vmlld0ludGVyYWN0aW9uLm1tCShyZXZpc2lvbiAxODM3NjMpCisrKyBT b3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvaW9zL1dLQ29udGVudFZpZXdJbnRlcmFjdGlvbi5tbQko d29ya2luZyBjb3B5KQpAQCAtODE4LDcgKzgxOCw3IEBAIC0gKENHUmVjdClfc2VsZWN0aW9uQ2xp cFJlY3QKIHsKICAgICBpZiAoX2Fzc2lzdGVkTm9kZUluZm9ybWF0aW9uLmVsZW1lbnRUeXBlID09 IElucHV0VHlwZTo6Tm9uZSkKICAgICAgICAgcmV0dXJuIENHUmVjdFplcm87Ci0gICAgcmV0dXJu IF9hc3Npc3RlZE5vZGVJbmZvcm1hdGlvbi5lbGVtZW50UmVjdDsKKyAgICByZXR1cm4gX3BhZ2Ut PmVkaXRvclN0YXRlKCkucG9zdExheW91dERhdGEoKS5zZWxlY3Rpb25DbGlwUmVjdDsKIH0KIAog LSAoQk9PTClnZXN0dXJlUmVjb2duaXplcjooVUlHZXN0dXJlUmVjb2duaXplciAqKXByZXZlbnRp bmdHZXN0dXJlUmVjb2duaXplciBjYW5QcmV2ZW50R2VzdHVyZVJlY29nbml6ZXI6KFVJR2VzdHVy ZVJlY29nbml6ZXIgKilwcmV2ZW50ZWRHZXN0dXJlUmVjb2duaXplcgpJbmRleDogU291cmNlL1dl YktpdDIvV2ViUHJvY2Vzcy9XZWJQYWdlL2lvcy9XZWJQYWdlSU9TLm1tCj09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0t IFNvdXJjZS9XZWJLaXQyL1dlYlByb2Nlc3MvV2ViUGFnZS9pb3MvV2ViUGFnZUlPUy5tbQkocmV2 aXNpb24gMTgzNzYzKQorKysgU291cmNlL1dlYktpdDIvV2ViUHJvY2Vzcy9XZWJQYWdlL2lvcy9X ZWJQYWdlSU9TLm1tCSh3b3JraW5nIGNvcHkpCkBAIC0xODYsNiArMTg2LDExIEBAIHZvaWQgV2Vi UGFnZTo6cGxhdGZvcm1FZGl0b3JTdGF0ZShGcmFtZSYKICAgICAgICAgcG9zdExheW91dERhdGEu aXNSZXBsYWNlQWxsb3dlZCA9IHJlc3VsdC5pc0NvbnRlbnRFZGl0YWJsZSAmJiAhcmVzdWx0Lmlz SW5QYXNzd29yZEZpZWxkICYmICFzZWxlY3RlZFRleHQuY29udGFpbnNPbmx5V2hpdGVzcGFjZSgp OwogICAgIH0KICAgICBpZiAoIXNlbGVjdGlvbi5pc05vbmUoKSkgeworICAgICAgICBpZiAobV9h c3Npc3RlZE5vZGUgJiYgbV9hc3Npc3RlZE5vZGUtPnJlbmRlcmVyKCkpIHsKKyAgICAgICAgICAg IEZyYW1lJiBlbGVtZW50RnJhbWUgPSBtX3BhZ2UtPmZvY3VzQ29udHJvbGxlcigpLmZvY3VzZWRP ck1haW5GcmFtZSgpOworICAgICAgICAgICAgcG9zdExheW91dERhdGEuc2VsZWN0aW9uQ2xpcFJl Y3QgPSBlbGVtZW50RnJhbWUudmlldygpLT5jb250ZW50c1RvUm9vdFZpZXcobV9hc3Npc3RlZE5v ZGUtPnJlbmRlcmVyKCktPmFic29sdXRlQm91bmRpbmdCb3hSZWN0KCkpOworICAgICAgICB9Cisg ICAgICAgIAogICAgICAgICBOb2RlKiBub2RlVG9SZW1vdmU7CiAgICAgICAgIGlmIChSZW5kZXJT dHlsZSogc3R5bGUgPSBFZGl0b3I6OnN0eWxlRm9yU2VsZWN0aW9uU3RhcnQoJmZyYW1lLCBub2Rl VG9SZW1vdmUpKSB7CiAgICAgICAgICAgICBDVEZvbnRSZWYgZm9udCA9IHN0eWxlLT5mb250Q2Fz Y2FkZSgpLnByaW1hcnlGb250KCkuZ2V0Q1RGb250KCk7Cg==