mozilla-mobile / focus-ios

⚠️ Firefox Focus (iOS) has moved to a new repository. It is now developed and maintained as part of: https://github.com/mozilla-mobile/firefox-ios
Mozilla Public License 2.0
1.26k stars 262 forks source link

No re-prompt on saving images to camera roll #672

Closed AaronMT closed 2 years ago

AaronMT commented 6 years ago

According to https://github.com/mozilla-mobile/focus-ios/issues/283 I should be re-prompted on saving images to camera roll.

Nothing.

marchiore commented 6 years ago

Hey @AaronMT,

I'm trying to solve this bug. First I readed the thread on the other issue (#283) and I noticed this PR (https://github.com/mozilla-mobile/focus-ios/pull/281) by @boek, that was merged on master.

But I can't see some files on my project like Blockzilla/Browser.swift either the changes on the other files.

I tried to reach someone at iRC, to help me with this but without success.

What i'm doing wrong?

AaronMT commented 6 years ago

You are not doing anything wrong.

I believe this code was refactored or just straight up removed with the https://github.com/mozilla-mobile/focus-ios/commit/6e1edeb555e7b1d3d34cba3336a125998bd7a63c#diff-93f8e8b1353c1dabb447526fe4178103 WKWebView migration (@boek ?)

(Also we're in #Focus on irc.mozilla.org)

marchiore commented 6 years ago

btw, I think this method is not being used on BrowserViewController.swift

fileprivate func presentImageActionSheet(title: String, link: String?, saveAction: @escaping () -> Void, copyAction: @escaping () -> Void) {
    let alertController = UIAlertController(title: title.truncated(limit: 160, position: .middle), message: nil, preferredStyle: .actionSheet)

    if let link = link {
        alertController.addAction(UIAlertAction(title: UIConstants.strings.copyLink, style: .default) { _ in
            UIPasteboard.general.string = link
        })

        alertController.addAction(UIAlertAction(title: UIConstants.strings.shareLink, style: .default) { _ in
            let activityViewController = UIActivityViewController(activityItems: [link], applicationActivities: nil)
            self.present(activityViewController, animated: true, completion: nil)
        })
    }

    alertController.addAction(UIAlertAction(title: UIConstants.strings.saveImage, style: .default) { _ in saveAction() })
    alertController.addAction(UIAlertAction(title: UIConstants.strings.copyImage, style: .default) { _ in copyAction() })
    alertController.addAction(UIAlertAction(title: UIConstants.strings.cancel, style: .cancel))

    alertController.popoverPresentationController?.permittedArrowDirections = UIPopoverArrowDirection.init(rawValue: 0)
    alertController.popoverPresentationController?.sourceView = self.view
    alertController.popoverPresentationController?.sourceRect = CGRect(x: self.view.bounds.size.width / 2.0, y: self.view.bounds.size.height / 2.0, width: 1.0, height: 1.0)
    present(alertController, animated: true, completion: nil)
}

thanks @AaronMT I joined the channel!

Sdaswani commented 6 years ago

Is this also an issue in the current release (3.10)?

Sdaswani commented 6 years ago

@AaronMT ???

marchiore commented 6 years ago

@Sdaswani, i’ve Tested on my phone with the last version of focus released on App Store and the problem happens too

boek commented 6 years ago

When we moved to WKWebView we used their native context menu over using our own. The big problem with this now is we aren't showing our own guard prompt in front of the system one. We'll have to go back to our own context menu to bring this functionality back :(

Sdaswani commented 6 years ago

OK no need to fix for 3.11, we'll handle it in triage.

AaronMT commented 6 years ago

That seems like an overall poor choice for code management. Maybe we should just disregard this feature.

bbinto commented 6 years ago

@boek could you please file a bug that adds a probe for users having the photo access denied. Let's decide then