Bug 144603

Summary: [iOS WK2] editorState should include information about selection clipping rectangle
Product: WebKit Reporter: Enrica Casucci <enrica>
Component: WebKit2Assignee: Enrica Casucci <enrica>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch cdumez: review+, cdumez: commit-queue-

Description Enrica Casucci 2015-05-04 15:57:26 PDT
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
Comment 1 Enrica Casucci 2015-05-04 16:03:05 PDT
Created attachment 252345 [details]
Patch
Comment 2 Chris Dumez 2015-05-04 16:05:18 PDT
Did you try a debug build or turning the assertions in postLayoutData() into RELEASE_ASSERTs?
Comment 3 Chris Dumez 2015-05-04 16:23:34 PDT
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.
Comment 4 Enrica Casucci 2015-05-04 16:50:40 PDT
(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.
Comment 5 Enrica Casucci 2015-05-04 16:52:57 PDT
Committed revision 183780.