Bug 226794 - Upstream AppHighlight Observer code
Summary: Upstream AppHighlight Observer code
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Megan Gardner
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-06-08 17:03 PDT by Megan Gardner
Modified: 2021-06-08 20:29 PDT (History)
4 users (show)

See Also:


Attachments
Patch (6.65 KB, patch)
2021-06-08 17:05 PDT, Megan Gardner
no flags Details | Formatted Diff | Diff
Patch for landing (7.04 KB, patch)
2021-06-08 18:02 PDT, Megan Gardner
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Megan Gardner 2021-06-08 17:03:42 PDT
Upstream AppHighlight Observer code
Comment 1 Megan Gardner 2021-06-08 17:05:42 PDT
Created attachment 430914 [details]
Patch
Comment 2 Tim Horton 2021-06-08 17:10:52 PDT
Comment on attachment 430914 [details]
Patch

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

> Source/WebKit/UIProcess/Cocoa/WebPageProxyCocoa.mm:70
> +#import <Synapse/SYNotesActivationObserver.h>

This will eventually need an SPI header

> Source/WebKit/UIProcess/Cocoa/WebPageProxyCocoa.mm:73
> +SOFT_LINK_CLASS_OPTIONAL(Synapse, SYNotesActivationObserver)

Oddly we don't really seem to deal with the optional-ness elegantly? Maybe it's OK because alloc returns nil and then -isVisible on nil returns NO and we updateAppHighlightsVisibility with NO? But it would be nice if it were an early return instead.

> Source/WebKit/UIProcess/WebPageProxy.h:3047
> +    void setUpHighlightsObserver();

There shouldn't be random methods in amongst the members, move this above.

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:9325
> +        if ([menuItem action] == @selector(createHighlightInCurrentGroupWithRange:))
> +            return;

Should this be checking for /either/ selector?
Comment 3 Wenson Hsieh 2021-06-08 17:16:12 PDT
rdar://79043357
Comment 4 Megan Gardner 2021-06-08 18:02:57 PDT
Created attachment 430929 [details]
Patch for landing
Comment 5 EWS 2021-06-08 20:29:25 PDT
Committed r278644 (238626@main): <https://commits.webkit.org/238626@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 430929 [details].