Bug 219635

Summary: [macOS] Change Universal Access zoom in the UI process
Product: WebKit Reporter: Per Arne Vollan <pvollan>
Component: WebKit Misc.Assignee: Per Arne Vollan <pvollan>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, darin, ews-watchlist, mifenton, sam, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
darin: review+
Patch
ews-feeder: commit-queue-
Patch none

Description Per Arne Vollan 2020-12-08 02:32:35 PST
Currently, the API UAZoomChangeFocus is called in the WebContent process when Universal Access zoom is enabled. To enable us to block the HI service com.apple.hiservices-xpcservice in the WebContent process, this call should be performed in the UI process.
Comment 1 Per Arne Vollan 2020-12-08 02:45:26 PST
Created attachment 415625 [details]
Patch
Comment 2 Darin Adler 2020-12-08 12:10:44 PST
Comment on attachment 415625 [details]
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.
Comment 3 Per Arne Vollan 2020-12-09 04:30:56 PST
Created attachment 415744 [details]
Patch
Comment 4 Per Arne Vollan 2020-12-09 04:52:44 PST
Created attachment 415745 [details]
Patch
Comment 5 EWS 2020-12-09 05:48:01 PST
Committed r270577: <https://trac.webkit.org/changeset/270577>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 415745 [details].
Comment 6 Radar WebKit Bug Importer 2020-12-09 05:48:16 PST
<rdar://problem/72134245>