1238807 - Twitter missing from Share menu on master

Status: RESOLVED FIXED

(Firefox for iOS :: Browser, defect)

Description

[James Hugman [:jhugman] [@jhugman]]

STR:
0. Ensure the twitter app is installed
1. Open a webpage in Firefox
2. Hit the share menu

Expected: 
Twitter to be available to share links to.

Observed:
Twitter is not available.

This looks like a regression of Bug 1228596.

It does not affect 1.4.

Comment 1

[Richard Newman [:rnewman]]

Needs a test!

Comment 2

[James Hugman [:jhugman] [@jhugman]]

Regression introduced by Bug 1206998.

Comment 3

[James Hugman [:jhugman] [@jhugman]]

Attachment: Pull request

URL as a string should not be in the list of activity items.

Comment 4

[Brian Nicholson (:bnicholson)]

(In reply to James Hugman [:jhugman] [@jhugman] from comment #3)
> Created attachment 8707854 [details] [review]
> Pull request
> 
> URL as a string should not be in the list of activity items.

Hm, but it looks like selectedURL is an NSURL. Why would adding an NSURL as a share item break sharing with Twitter? Moreover, if you remove the URL from the share items, what are we even sharing to Twitter? Also, does this regress anything from bug 1206998? Just curious why it was added in the first place.

Sorry for all the questions -- would like to understand the fix a bit better before giving r+.

Comment 5

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Comment on attachment 8707854 [details] [review]
Pull request

This patch looks good to me. Tested with Twitter, Mail, Copy, 1Password, Things.

Comment 6

[James Hugman [:jhugman] [@jhugman]]

This is /exactly/ the same problem as seen in Bug 1228596.

That fix worked because self will return the selected URL via UIActivityItemSource protocol. The bug existed because extensions set the rule of how many activity items they can support, and a common rule is that they can interpret exactly one of activity items. Adding both a URL and a UIActivityItemSource means those items match more than one, so the extension doesn't show up.

The bug was introduced in bug 1206998, which makes the ShareExtensionHelper work with either a tab or a URL.

In my testing this didn't break any of the Share Link behaviour, though I'm not convinced that all the share menu items are going to be valid for that use case, (e.g. Share Link -> 1Password).

Comment 7

[Brian Nicholson (:bnicholson)]

Comment on attachment 8707854 [details] [review]
Pull request

Got it. Thanks for the explanation!

Comment 8

[James Hugman [:jhugman] [@jhugman]]

Merged.