| Summary: | REGRESSION (PSON): Firefox app lacks Open in New Tab in menu | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Tim Horton <thorton> | ||||||||
| Component: | New Bugs | Assignee: | Tim Horton <thorton> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | ap, bdakin, beidson, benjamin, cdumez, cmarcelo, commit-queue, dbates, ews-watchlist, garvankeeley, ggaren, simon.fraser, webkit-bug-importer, wenson_hsieh | ||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||
| Version: | WebKit Nightly Build | ||||||||||
| Hardware: | Unspecified | ||||||||||
| OS: | Unspecified | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Tim Horton
2019-01-11 13:53:16 PST
Created attachment 358940 [details]
Patch
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 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 And fix the macOS build Created attachment 358943 [details]
Patch
Created attachment 358946 [details]
Patch
Comment on attachment 358946 [details] Patch Clearing flags on attachment: 358946 Committed r239880: <https://trac.webkit.org/changeset/239880> All reviewed patches have been landed. Closing bug. On Firefox iOS, installing the custom context menu is still broken when built with XCode 10.2 beta 4. 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. (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 :) 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 |