Bug 193366 – REGRESSION (PSON): Firefox app lacks Open in New Tab in menu

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

[Tim Horton]

REGRESSION (PSON): Firefox app lacks Open in New Tab in menu

[Tim Horton]

Created attachment 358940 [details]
Patch

[Tim Horton]

<rdar://problem/46097212>

[Simon Fraser (smfr)]

Comment on attachment 358940 [details]
Patch

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

> Source/WTF/wtf/spi/darwin/dyldSPI.h:71
> +#define DYLD_IOS_VERSION_FIRST_WITH_LAZY_GESTURE_RECOGNIZER_INSTALLATION 0
> +#define DYLD_IOS_VERSION_FIRST_WITH_PROCESS_SWAP_ON_CROSS_SITE_NAVIGATION 0

These don't seem like dyldSPI

[Tim Horton]

Comment on attachment 358940 [details]
Patch

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

>> Source/WTF/wtf/spi/darwin/dyldSPI.h:71
>> +#define DYLD_IOS_VERSION_FIRST_WITH_PROCESS_SWAP_ON_CROSS_SITE_NAVIGATION 0
> 
> These don't seem like dyldSPI

True. I'll move them to an #else in VersionChecks.h

[Tim Horton]

And fix the macOS build

[Tim Horton]

Created attachment 358943 [details]
Patch

[Tim Horton]

Created attachment 358946 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 358946 [details]
Patch

Clearing flags on attachment: 358946

Committed r239880: <https://trac.webkit.org/changeset/239880>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Garvan Keeley]

On Firefox iOS, installing the custom context menu is still broken when built with XCode 10.2 beta 4.

[Garvan Keeley]

Attempting to re-install the long press gesture recognizer after each URL change detected works in *some* cases:
https://github.com/mozilla-mobile/firefox-ios/commit/daf2a5a58e04fffc8f7b8a67e766d037ed5960bf

(It may not be clear from that diff, but that code runs on WKWebView URL change events)

Basically, if you have anything prescriptive we should be doing it would help us greatly.

[Tim Horton]

(In reply to Garvan Keeley from comment #11)
> Attempting to re-install the long press gesture recognizer after each URL
> change detected works in *some* cases:
> https://github.com/mozilla-mobile/firefox-ios/commit/
> daf2a5a58e04fffc8f7b8a67e766d037ed5960bf
> 
> (It may not be clear from that diff, but that code runs on WKWebView URL
> change events)
> 
> Basically, if you have anything prescriptive we should be doing it would
> help us greatly.

I don't think there's a great solution to this problem. The existing solution already isn't great, because you're digging into WKWebView internals, which are not stable (as we saw in this bug).

The two things you want to observe are 1) any time we swap processes, and any time the WKWebView is added to the view hierarchy. I can't prescribe a solution since it's almost certainly going to involve more dirty internals, and I can't condone that! But I'm sure you can come up with something that is workable :)

I'd also recommend filing a radar requesting proper API for whatever you're trying to do, so we can get out of this business altogether.

I'll give you one hint, though: the thing you want to run after is -[WKContentViewInteraction setupInteraction]. Maybe take a peek at the callers :)

[Garvan Keeley]

Thanks for the tips! I'll poke around and see if I can come up with a more robust hack. Being that hacks is all I have, I filed a request for official support:
https://bugreport.apple.com/web/?problemID=48936921