[webkit-reviews] review granted: [Bug 59121] WebKit2: Text input is largely broken when there are subframes loading : [Attachment 202008] proposed fix

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Alexey Proskuryakov <ap at webkit.org>'s
request for review:
Bug 59121: WebKit2: Text input is largely broken when there are subframes
loading
https://bugs.webkit.org/show_bug.cgi?id=59121

Attachment 202008: proposed fix
https://bugs.webkit.org/attachment.cgi?id=202008&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=202008&action=review


review+ as long as you do not add a member function to Frame!

> Source/WebCore/page/Frame.h:110
> +	   void willTransitionToCommitted();

We should not add a function to Frame for this. The code should be in
FrameLoader. It’s completely reasonable for FrameLoader to make Editor calls.
You can put this new function in either FrameLoader or Editor, but not in
Frame.

> Source/WebKit2/UIProcess/WebPageProxy.cpp:3070
> +    // This is a temporary state when editing. Flipping secure input state
too quickly can expose race conditions.
> +    if (couldChangeSecureInputState && !editorState.selectionIsNone)
> +	   m_pageClient->updateSecureInputState();

I don’t understand the comment. It doesn’t say what it’s referring to. What is
a temporary state when editing? What does “too quickly” mean? It seems like you
moved this comment here, but I don’t understand it in its original location.