Add FXIOS-6047 - iOS No Longer Asking For Permission To Paste URL

[tisumi99]

📜 Tickets

Jira ticket
Github issue

💡 Description

Replaced UIButton with UIPasteControl in URL toast for iOS 16 and above. This fixes the attached issue as the app will no longer read the clipboard without user interaction. Clicking on UIPasteControl gives the app permission to read the clipboard. Issue does not exist prior to iOS 16 as UIPasteControl and the clipboard security measures were released in iOS 16.

• Added subclass PasteControlToast: ButtonToast. Subclass replaces UIButton with UIPasteControl.

• Added method BrowserController.paste(). This method is called after UIPasteControl is cliked. Method gets the URL from the clipboard and opens a new browser tab with the URL.

• Added method BrowserViewController.shouldDisplay() as delegate for ClipboardBarDisplayHandler. Method sets view.pasteConfiguration and configures URL toast.

• Added branched code to ClipboardBarDisplayHandler.checkIfShouldDisplayBar() to call BrowserViewController.shouldDisplay() instead of BrowserViewController.shouldDisplay(clipBoardURL:) if iOS 16 or greater. The branched code does not read the clipboard as that would trigger iOS to ask for user permission. Instead, app waits until UIPasteControl is clicked to read the clipboard.

• Added method ClipboardBarDisplayHandler.shouldDisplayBar(pasteBoardChangeCount:). Since app cannot read the clipboard without iOS asking for user permission, app is now using UIPasteboard.general.changeCount to check if the clipboard contents have changed. Checking changeCount does not required user permission.

📝 Checklist

You have to check all boxes before merging

• Filled in the above information (tickets numbers and description of your work)
• Updated the PR name to follow our PR naming guidelines
• Wrote unit tests and/or ensured the tests suite is passing
• When working on UI, I checked and implemented accessibility (minimum Dynamic Text and VoiceOver)
• If needed, I updated documentation / comments for complex code and public methods
• If needed, added a backport comment (example @Mergifyio backport release/v120)

[tisumi99]

Before - iOS Asks User Permission Repeatedly

Screen.Recording.2024-05-10.at.10.04.32.AM.mp4

After - iOS No Longer Asking Permission

Screen.Recording.2024-05-10.at.10.13.26.AM.mov

[mattreaganmozilla]

Thanks for this PR @tisumi99! Can you clarify the before/after UI, it looks like a few changes have been made (for ex. we're no longer displaying the destination URL to the user in the toast descriptionText, and the button has changed etc.). These changes seem fine but they're not listed in the ticket and so I was hoping to just confirm where these decisions on the UI originated from?

[tisumi99]

@mattreaganmozilla
We can no longer display the URL on the toast as the app should not read the pasteboard before the user clicks on UIPasteControl.

The UI for UIPasteControl is provided by Apple and is not very customizable. The background color, foreground color, and corner radius are configurable and set to the current theme. However, the "Paste" text and font cannot be changed. We do have the option of not displaying the icon next to "Paste" but I left the icon as that seems to be the default and users may already be familiar with the icon from seeing it in other apps.

This blog post does a pretty job summarizing the pasteboard security changes that happened in iOS16:
https://sarunw.com/posts/uipasteboard-privacy-change-ios16/

Links to UIPasteControl documentation for convenience:
https://developer.apple.com/documentation/uikit/uipastecontrol
https://developer.apple.com/documentation/uikit/uipastecontrol/configuration

[mattreaganmozilla]

Thanks @tisumi99, I understand the rationale for the changes, just wanted to clarify if there had been discussions outside of the ticket on the UI changes, since the update here differs from the expected behavior listed in the ticket afaics (though I don't have 100% context on that). This LGTM though, I'm reaching out to the team to confirm if there are any other steps that may be needed for this UI update.

[tisumi99]

@mattreaganmozilla
There were no discussions outside of the ticket.

I originally did have a fix that did as the ticket asked for. I had ClipboardBarDisplayHandler register a nil from the pasteboard to mean that the user did not give permission to read the pasteboard as there was no other way to know if the user denied permission. Once the nil was registered, ClipboardBarDisplayHandler would never try to read the pastboard again until the app was restarted. This felt like a work around to deal with the iOS 16 changes and users would have to restart the app if they decided later that they do want to paste URLs. Also in the case where the user gave permission, the user would have to double confirm that they wanted to paste the url (once in the iOS alert, and again in the toast). So it seemed to make more sense to implement UIPasteControl to 1) address the ticket, 2) address the double confirm issue, and 3) conform to the updated pasteboard security protocol.

The only other option I can think of is to not make any changes to the app and let users know that they have to give Firefox permission to read the pasteboard in iOS settings to avoid constantly being asked for permission. Asking for permission each time the pasteboard is read is the default setting.

[mattreaganmozilla]

Thanks again @tisumi99 for your work on this. 👍 The change makes sense to me (and thank you for the well-written PR description and screenshots). I'm just double-checking a couple things, should have it reviewed shortly.

[mattreaganmozilla]

@tisumi99 Would you mind rebasing this branch against current main to see if that fixes the CI failure? Thank you

[mattreaganmozilla]

Hi @tisumi99, it looks like something may have gone awry with the the rebase you made (diff is now showing 289 files changed). Can you please try to correct it? If you run into further difficulties you can alternatively open a new PR if needed (cherry-pick your original changes to it, and then just message me the new PR link here), and we can close this one.

[tisumi99]

@mattreaganmozilla
Oops, first time doing a rebase. I fixed the PR.

[mobiletest-ci-bot]

	Warnings
⚠️	Variable 'crossCircleFill' in Medium is out of alphabetical order.
⚠️	Variable 'cross' in Medium is out of alphabetical order.

	Messages
📖	Project coverage: 32.31%
📖	Edited 3 files
📖	Created 0 files

Client.app: Coverage: 30.92

File	Coverage
BrowserViewController.swift	4.43%	⚠️
ButtonToast.swift	0.0%	⚠️
ClipboardBarDisplayHandler.swift	0.0%	⚠️

Generated by 🚫 Danger Swift against 30a0d13

[mattreaganmozilla]

Thanks @tisumi99 for your work on this, LGTM. 👍 I'm just confirming the UI change with the UX team and then I'll merge.

[mattreaganmozilla]

@tisumi99 Just wanted to let you know I haven't forgotten about this PR, I'm waiting to get confirmation on the UI changes with our UX team which is something we try to do whenever UI is updated. As soon as I hear back I'll update. Thanks again for this PR.

[tisumi99]

Thanks for the update @mattreaganmozilla .

[mattreaganmozilla]

@tisumi99 Still waiting for confirmation from UX team (they like to review UI changes whenever possible before merging). That process can sometimes take a while, but I'll update as soon as I hear back. Thank you