[webkit-reviews] review granted: [Bug 219635] [macOS] Change Universal Access zoom in the UI process : [Attachment 415625] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Per Arne Vollan <pvollan at apple.com>'s
request for review:
Bug 219635: [macOS] Change Universal Access zoom in the UI process
https://bugs.webkit.org/show_bug.cgi?id=219635

Attachment 415625: Patch

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




--- Comment #2 from Darin Adler <darin at apple.com> ---
Comment on attachment 415625
  --> https://bugs.webkit.org/attachment.cgi?id=415625
Patch

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

I think we are moving too much of the code from shared between WebKit and
WebKitLegacy to have two copies.

> Source/WebKit/UIProcess/mac/WebPageProxyMac.mm:717
> +void WebPageProxy::changeUniversalAccessZoomFocus(const WebCore::IntRect&
viewRect, const WebCore::IntRect& selectionRect)

Can we find a way to share this code between WebKit and WebKitLegacy? Maybe put
a function in WebCore they both call? It’s annoying to have two copies.

> Source/WebKit/UIProcess/mac/WebPageProxyMac.mm:727
> +    NSRect nsCaretRect = NSMakeRect(selectionRect.x(), selectionRect.y(),
selectionRect.width(), selectionRect.height());
> +    NSRect nsViewRect = NSMakeRect(viewRect.x(), viewRect.y(),
viewRect.width(), viewRect.height());
> +    nsCaretRect = toUserSpaceForPrimaryScreen(nsCaretRect);
> +    nsViewRect = toUserSpaceForPrimaryScreen(nsViewRect);
> +    CGRect cgCaretRect = NSRectToCGRect(nsCaretRect);
> +    CGRect cgViewRect = NSRectToCGRect(nsViewRect);

I know this is just moved code, but I suggest this alternative way of writing
it:

    auto cgCaretRect =
NSRectToCGRect(toUserSpaceForPrimaryScreen(selectionRect));
    auto cgViewRect = NSRectToCGRect(toUserSpaceForPrimaryScreen(viewRect));

This should compile and do the same as the code above. If not, I’d like to know
why not.

> Source/WebKitLegacy/mac/WebCoreSupport/WebChromeClient.mm:1185
> +    NSRect nsCaretRect = NSMakeRect(selectionRect.x(), selectionRect.y(),
selectionRect.width(), selectionRect.height());
> +    NSRect nsViewRect = NSMakeRect(viewRect.x(), viewRect.y(),
viewRect.width(), viewRect.height());
> +    nsCaretRect = toUserSpaceForPrimaryScreen(nsCaretRect);
> +    nsViewRect = toUserSpaceForPrimaryScreen(nsViewRect);
> +    CGRect cgCaretRect = NSRectToCGRect(nsCaretRect);
> +    CGRect cgViewRect = NSRectToCGRect(nsViewRect);

Ditto.