Bug 198451 – Remove some unused selection code

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Tim Horton]

Remove some unused selection code

[Tim Horton]

Created attachment 371111 [details]
Patch

[Tim Horton]

Created attachment 371251 [details]
Patch

[Megan Gardner]

Comment on attachment 371251 [details]
Patch

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

I’m nervous about this, but the bots seem to be green....

> Source/WebKit/Platform/spi/ios/UIKitSPI.h:-559
> -

Are we sure non of this is needed? I thought this assistant was still used in legacy web kit, though it’s mostly broken, it can still be triggered sometimes.

> Source/WebKit/Platform/spi/ios/UIKitSPI.h:-600
> -- (void)showDictionaryFor:(NSString *)selectedTerm fromRect:(CGRect)presentationRect;

I know we use some of the above methods, I’m surprised that these are ok to remove? Are we sure this is ok?

> Source/WebKit/Shared/ios/GestureTypes.h:45
>      PhraseBoundary

Does this need to have values specified, or since It’s an enumeration class jf’s Ok? Removing one in the middle of a list makes me nervous.

> Source/WebKit/UIProcess/ios/WKPDFView.mm:526
>      auto selectionAssistant = adoptNS([[UIWKSelectionAssistant alloc] initWithView:[_hostViewController view]]);

File a radar? I would hope this would be very easy to do, and would clean up a lot.

[Wenson Hsieh]

Comment on attachment 371251 [details]
Patch

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

>> Source/WebKit/Platform/spi/ios/UIKitSPI.h:-559
>> -
> 
> Are we sure non of this is needed? I thought this assistant was still used in legacy web kit, though it’s mostly broken, it can still be triggered sometimes.

This is WebKit

[Wenson Hsieh]

Comment on attachment 371251 [details]
Patch

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

>>> Source/WebKit/Platform/spi/ios/UIKitSPI.h:-559
>>> -
>> 
>> Are we sure non of this is needed? I thought this assistant was still used in legacy web kit, though it’s mostly broken, it can still be triggered sometimes.
> 
> This is WebKit

Oops, half-finished comment 🤦🏻‍♂️

This is just a forward declaration in WebKit, so I don't think it will have any impact in legacy WebKit.

>> Source/WebKit/UIProcess/ios/WKPDFView.mm:526
>>      auto selectionAssistant = adoptNS([[UIWKSelectionAssistant alloc] initWithView:[_hostViewController view]]);
> 
> File a radar? I would hope this would be very easy to do, and would clean up a lot.

+1

[WebKit Commit Bot]

Comment on attachment 371251 [details]
Patch

Clearing flags on attachment: 371251

Committed r246086: <https://trac.webkit.org/changeset/246086>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/51419978>

[WebKit Commit Bot]

Re-opened since this is blocked by bug 198549

[Tim Horton]

Tried again in https://trac.webkit.org/changeset/246116/webkit