| Summary: | Upstream AppHighlight Observer code | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Megan Gardner <megan_gardner> | ||||||
| Component: | New Bugs | Assignee: | Megan Gardner <megan_gardner> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | hi, thorton, webkit-bug-importer, wenson_hsieh | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
|
Description
Megan Gardner
2021-06-08 17:03:42 PDT
Created attachment 430914 [details]
Patch
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? Created attachment 430929 [details]
Patch for landing
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]. |