[webkit-reviews] review denied: [Bug 136729] Latching in iframes is not working as expected : [Attachment 238381] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has denied Brent Fulgham
<bfulgham at webkit.org>'s request for review:
Bug 136729: Latching in iframes is not working as expected
https://bugs.webkit.org/show_bug.cgi?id=136729

Attachment 238381: Patch
https://bugs.webkit.org/attachment.cgi?id=238381&action=review

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=238381&action=review


> Source/WebCore/page/MainFrame.cpp:39
> +#if PLATFORM(COCOA)
> +    , m_latchedState(std::make_unique<ScrollLatchedState>())
> +#endif

COCOA includes iOS, and I don't think you want that.

> Source/WebCore/page/MainFrame.h:47
> +#if PLATFORM(COCOA)

MAC?

> Source/WebCore/page/mac/EventHandlerMac.mm:819
> +static bool isWK1EventForOtherPlatformFrame(const Frame& frame)

We try really hard to avoid explicitly distinguishing between WK1 and WK2 in
WebCore.

The name of this function needs to communicate WHY WK1 needs different
handling.

> Source/WebCore/page/scrolling/ScrollLatchedState.h:37
> +class ScrollLatchedState {

I'd prefer ScrollLatchingState