Closed Bug 1153285 Opened 7 years ago Closed 7 years ago

Put "Open In New Tab" back as a context menu item

Categories

(Firefox for iOS :: General, defect)

ARM
iOS 8
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Tracking Status
fennec + ---

People

(Reporter: aaronmt, Assigned: bnicholson)

References

Details

(Whiteboard: noteworthy)

Attachments

(1 file)

We should match Firefox for Android behaviour here, long tap on links, 'Open in New Tab' addTab() should open them in the background
tracking-fennec: ? → +
Assignee: nobody → sarentz
Summary: Open in New Tab should open new tabs in the background → Open fro the WKWebView Action Sheet should open new tabs in the background
Summary: Open fro the WKWebView Action Sheet should open new tabs in the background → Open from the WKWebView Action Sheet should open new tabs in the background
After looking at this for a while, mostly reading WebKit code, I don't think there is a simple solution for this that will not involve private APIs and patching UIKit/WebKit methods.

I think we should leave this menu as it is now and file a bigger bug for post v1 to re-implement it in our own code, fixing all the regressions that we encountered when we previously tried.

I am marking this as tracking-? so that we can discuss this during triage.
More technical:

Using the Open action from the action sheet that appears when you long press on a link results in the  - webView:decidePolicyForNavigationAction:decisionHandler: being called.

But, from the WKNavigationAction we cannot see *how* that link was opened. We don't know if it was a regular tap on a link, or if the user went through the action sheet. We miss that context.

We can *maybe* hack around that by patching UIAlertView or WKWebView and inject our own code, but I think that is fragile and probably 'private API usage' in App Store terms.
This hurts, but let's discuss.
tracking-fennec: + → ?
st3fan, just to be clear what's the NavigationType for the decidePolicyForNavigation [1]? I would expect its Other, but your message makes it sound like its actually LinkActivated?

https://developer.apple.com/library/ios/documentation/WebKit/Reference/WKNavigationAction_Ref/index.html#//apple_ref/swift/enum/WKNavigationType
Still investigating, but we'll track it.
tracking-fennec: ? → +
I'll take a look at this.
Assignee: sarentz → bnicholson
Status: NEW → ASSIGNED
Well, bad news: user content scripts are not injected into iframes, so we'll still have bug 1127606. It looks like we can't do much better than the first implementation we tried in bug 1109666.
Not sure how I missed it, but Stefan pointed me to initWithSource:injectionTime:forMainFrameOnly:, which is exactly what I was looking for.
Summary: Open from the WKWebView Action Sheet should open new tabs in the background → Put "Open In New Tab" back as a context menu item
Comment on attachment 8609653 [details] [review]
Link to Github pull-request: https://github.com/mozilla/firefox-ios/pull/494

I tested a bunch. Works great! Just some questions in the PR comments.
Attachment #8609653 - Flags: review?(sarentz) → review+
Going to look into element highlighting/Open In Background as follow-ups.

https://github.com/mozilla/firefox-ios/commit/ab9b821104a4b32f067f7d4b8ae6d22f0c54e0b1
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Depends on: 1169075
Depends on: 1170696
Whiteboard: noteworthy
You need to log in before you can comment on or make changes to this bug.