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

fix: ignore sensitive flag when there is no summary or media #1145

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

danielroe
Copy link
Member

Mastodon uses sensitive flag + empty spoiler text to indicate sensitive media.

resolves #1072

@danielroe danielroe added the c: bug Something isn't working label Jan 15, 2023
@danielroe danielroe self-assigned this Jan 15, 2023
@stackblitz
Copy link

stackblitz bot commented Jan 15, 2023

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@netlify
Copy link

netlify bot commented Jan 15, 2023

Deploy Preview for elk-docs canceled.

Name Link
🔨 Latest commit fbbbec3
🔍 Latest deploy log https://app.netlify.com/sites/elk-docs/deploys/63c34e8a814fce000b820c58

@netlify
Copy link

netlify bot commented Jan 15, 2023

Deploy Preview for elk-zone ready!

Name Link
🔨 Latest commit fbbbec3
🔍 Latest deploy log https://app.netlify.com/sites/elk-zone/deploys/63c34e8a35be7d00099bcd60
😎 Deploy Preview https://deploy-preview-1145--elk-zone.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@rshigg
Copy link
Contributor

rshigg commented Jan 15, 2023

Are spoilerText and mediaAttachments the only two things that can appear inside the hidden area?

@danielroe
Copy link
Member Author

You can also have a poll, but I don't believe it's possible to mark a poll as sensitive.

@rshigg
Copy link
Contributor

rshigg commented Jan 15, 2023

Sounds good. I think since the condition has gotten quite long it should be stored in a variable to help with readability and to make it easier to modify in the future if needed.

@TheEnbyperor
Copy link

I don’t think this should be merged. The activitypub spec allows a post to be marked as sensitive without a summary nor media attachments.

Mastodon may never itself generate a post like this, but a federated posts may have this.

Elk should instead have a way of displaying posts as sensitive (i.e. hiding behind some filler CW) regardless of a CW or media existing.

@rshigg
Copy link
Contributor

rshigg commented Jan 15, 2023

That's a great point! I guess the real solution here is to update the verbiage to be less confusing when this specific edge case happens.

@danielroe
Copy link
Member Author

danielroe commented Jan 15, 2023

The tricky thing is that Mastodon uses this: sensitive: true with no spoiler text to refer specifically to displaying the status but not the images. This may be a bug or failure to comply with the protocol but it is how the Mastodon web client currently behaves.

What about this set of behaviour?

  • senstive: true but no spoiler text or image - hide entire status
  • sensitive: true but no spoiler text, but an image - hide only image
  • sensitive: true with spoiler text - hide entire status behind spoiler text

@danielroe danielroe marked this pull request as draft January 15, 2023 01:26
@ayoayco
Copy link
Member

ayoayco commented May 7, 2023

Hi all! I think we can close this now as PRs #2065 and #2072 will address this, and we will have further improvement on #2068

cc @danielroe @patak-dev

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: bug Something isn't working
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

Sensitive flag should be ignored if summary == null
4 participants