[webkit-reviews] review granted: [Bug 230204] [macOS] Avoid computing post-layout EditorState data unless necessary : [Attachment 438144] For EWS

[bugzilla-daemon at webkit.org]

Devin Rousso <drousso at apple.com> has granted Wenson Hsieh
<wenson_hsieh at apple.com>'s request for review:
Bug 230204: [macOS] Avoid computing post-layout EditorState data unless
necessary
https://bugs.webkit.org/show_bug.cgi?id=230204

Attachment 438144: For EWS

https://bugs.webkit.org/attachment.cgi?id=438144&action=review




--- Comment #5 from Devin Rousso <drousso at apple.com> ---
Comment on attachment 438144
  --> https://bugs.webkit.org/attachment.cgi?id=438144
For EWS

View in context: https://bugs.webkit.org/attachment.cgi?id=438144&action=review

r=me, neat stuff :)

> Source/WebKit/WebProcess/WebPage/WebPage.cpp:1234
> -    result.isContentEditable = selection.isContentEditable();
> +    result.isContentEditable = selection.hasEditableStyle();

These appear to have different `UserSelectAllTreatment`.  Is that a problem?

> Source/WebKit/WebProcess/WebPage/WebPage.cpp:1327
> +    auto isEditable = frame->selection().selection().hasEditableStyle() ?
EditorStateIsContentEditable::Yes : EditorStateIsContentEditable::No;

ditto (:1231)

> Source/WebKit/WebProcess/WebPage/WebPage.cpp:3561
>      m_hasEverFocusedElementDueToUserInteractionSincePageTransition = false;

NIT: Shouldn't this really be inside a `HAVE(TOUCH_BAR)`?

> Source/WebKit/WebProcess/WebPage/WebPage.cpp:5979
> -    m_hasEverFocusedElementDueToUserInteractionSincePageTransition |=
m_userIsInteracting;
> +    if (m_userIsInteracting && m_focusedElement)
> +	   m_hasEverFocusedElementDueToUserInteractionSincePageTransition =
true;

This seems like a actual functional difference.  Is it something we can/should
test?

> Source/WebKit/WebProcess/WebPage/mac/WebPageMac.mm:1089
> +    if (m_needsFontAttributes)

NIT: it'd be nice if we could add comments explaining why these things require
computing post layout data