Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(features/home/details): download c2pa file #3244

Merged
merged 4 commits into from
May 14, 2024

Conversation

olgahaha
Copy link
Contributor

@olgahaha olgahaha commented May 10, 2024

Added

  1. Update the share menu in the asset detail page as follows: #3244
    • Add "Download C2PA" (new option)
    • Add "View Blockchain Proof" (old: View asset profile)
    • Update "Share Blockchain Proof" (current: Share Asset Profile)
    • Update "Copy Nid" (current: Copy IPFS address)


┆The issue is also created on Asana

Update the share menu in the asset detail page as follows:
- Add "Download C2PA" (new option)
- Change "View asset profile" to "View Blockchain Proof"
  (old: View asset profile)
- Change "Share Asset Profile" to "Share Blockchain Proof"
  (current: Share Asset Profile)
- Change "Copy IPFS address" to "Copy Nid" (current: Copy IPFS address)
@olgahaha olgahaha requested a review from shc261392 May 10, 2024 11:37
@olgahaha olgahaha marked this pull request as draft May 10, 2024 11:38
@olgahaha olgahaha marked this pull request as ready for review May 10, 2024 12:12
@olgahaha olgahaha marked this pull request as draft May 10, 2024 12:16
@olgahaha olgahaha marked this pull request as ready for review May 10, 2024 12:47
src/app/features/home/details/details.page.ts Outdated Show resolved Hide resolved
Comment on lines +577 to +584
await ShareService.shareFile(fileUrl)
.catch(async reason => {
if (reason?.message !== 'Share canceled') {
await this.errorService.toastError$(reason).toPromise();
}
})
.finally(() => Filesystem.deleteFile({ path: filePath }));
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible that on some sharing target platforms, shared files (especially image) are compressed and lost the C2PA injection data? As far as I know it happens at least on Slack and some major social media platforms.

Upload an C2PA image to slack -> download uploaded file -> get a file with different checksum and no C2PA claim

Would such situation happen when using the shareFile function? If yes we could probably consider sharing the IPFS url or add some warning about possible target platform compression.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a strip metadata mechanism on Slack. However, since the URL has just been added but not pinned to IPFS, I think it's better to download it immediately to prevent it from being garbage collected. Additionally, besides sharing on social media, the function can also be shared to app files or emails, so social media might be considered a limitation. I'll ask about it in the task, and we could update it in the future sprint if the UI or text should be modified.

@olgahaha olgahaha requested a review from shc261392 May 13, 2024 06:18
@olgahaha olgahaha merged commit f3bd214 into master May 14, 2024
9 checks passed
@olgahaha olgahaha deleted the feature-asset-download-c2pa branch May 14, 2024 02:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants