-
Notifications
You must be signed in to change notification settings - Fork 213
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
Implementation plan: Blurring sensitive content in the frontend #2118
Conversation
Full-stack documentation: https://docs.openverse.org/_preview/2118 Please note that GitHub pages takes a little time to deploy newly pushed code, if the links above don't work or you see old versions, wait 5 minutes and try again. You can check the GitHub pages deployment action list to see the current status of the deployments. |
See also #2118 for an exploration of how the feature-flags implementation can be presented to the users as preferences. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is very close to LGTM, there are two things I would like to see addressed:
From the project proposal:
Sensitive results on the search results page are blurred. Users can unblur and reblur individual results through a simple and clear interaction on the search results page. In addition to being able to opt-in to having sensitive results included in their query, users can disable blurring by default. This parameter is stored in the application state. If users disable blurring by default, there is not an option to “reblur” images.
I don't see mention of this state in this implementation plan. It seems we might need to store a list of "unblurred" IDs in app storage.
Some mention of these analytics events would be useful as well.
...ecting_sensitive_textual_content/20230506-implementation_plan_frontend_blurring_sensitive.md
Outdated
Show resolved
Hide resolved
Co-authored-by: Zack Krida <[email protected]>
Drafted to address review suggestions. |
...ecting_sensitive_textual_content/20230506-implementation_plan_frontend_blurring_sensitive.md
Show resolved
Hide resolved
...ecting_sensitive_textual_content/20230506-implementation_plan_frontend_blurring_sensitive.md
Show resolved
Hide resolved
|
Oops, I quoted too much @dhruvkb, I just meant this part:
It's a fairly minor question, but when an image is unblurred, where do we store that state, so that navigating from a single result back to the search results, all the images a user has manually un-blurred remain so? |
Oooh, I misread and misinterpreted that to mean the view/hide button on the single result page. IIRC such an interaction is also not present in the design mockups. To enable that interaction, the IDs of images unblurred in a session can be stored in |
I've removed myself from the reviewers list and requested a review from @sarayourfriend . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM so far. There is one thing missing, which is the usage of the sensitivity field from the API to show different text on the single result page.
Here is the link to the disclaimer in the designs: https://www.figma.com/file/bDTfGzBit8rGRPktOR1cu4/Safe-content-browsing-flow?type=design&node-id=1140-27389&t=6IBY0jhgf8sSVwCs-4
The designs themselves don't make it clear that this is the case aside from the implication in the text, but here is the relevant comment from the original design issue: #791 (comment)
It's probably a mostly trivial feature aside from the fact that we won't have information about whether providers have marked something as sensitive because we don't ingest anything that is marked sensitive by providers at the moment. So the only two possible pieces of copy (three if you count the combination) is that Openverse user's reported the content as sensitive and that we detected sensitive text.
@fcoveram Can you clarify something in the designs? The words in pink in the disclaimer: are they meant to be links or just emphasised? I vaguely recall the idea that we'd write a page describing how content sensitivity is determined and moderated, but we actually don't have any work set aside for that, neither in this implementation plan nor explicitly in the project proposal.
@dhruvkb Regardless of whether it was part of the original discussion, I think we should indeed have such a page, especially to describe what we mean by sensitive textual content. We should explain clearly the huge caveats in our process, it's extreme naïveté and imperfectness and that we're actively iterating to make it better but chose a simpler approach to begin with to improve accessibility sooner than later. If this IP included one more step to add such a page, without needing to write much of anything specific about the text in this IP (leaving that up to the implementing PR), that would be sufficient.
...ecting_sensitive_textual_content/20230506-implementation_plan_frontend_blurring_sensitive.md
Outdated
Show resolved
Hide resolved
...ecting_sensitive_textual_content/20230506-implementation_plan_frontend_blurring_sensitive.md
Show resolved
Hide resolved
...ecting_sensitive_textual_content/20230506-implementation_plan_frontend_blurring_sensitive.md
Outdated
Show resolved
Hide resolved
...ecting_sensitive_textual_content/20230506-implementation_plan_frontend_blurring_sensitive.md
Outdated
Show resolved
Hide resolved
...ecting_sensitive_textual_content/20230506-implementation_plan_frontend_blurring_sensitive.md
Outdated
Show resolved
Hide resolved
...ecting_sensitive_textual_content/20230506-implementation_plan_frontend_blurring_sensitive.md
Outdated
Show resolved
Hide resolved
…epping in Co-authored-by: sarayourfriend <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
Links to internal pages. The idea comes from this comment (quote)
You're right that it wasn't considered in the planning or implementation. I think we should add it and ask comms folks for help on the best way to put it down. |
@dhruvkb please create the milestone and issues for this implementation so that work can commence on it. Once you've done so, can you update the project thread with a link to the new milestone under the "Issues" heading? Thanks. |
Created the milestone#12 and linked it to the project thread. I'll create and add issues to it. |
@dhruvkb Can you confirm that you are still planning to create the issues for this implementation plan? If you're not able to do so that I can delegate it to someone else or do it myself. I see two issues to do in the milestone, but the IP makes it seem like there would be far more than two issues (step 1 probably needs an issue per component and the other two steps need their own). Is my understanding of the intended issues accurate? |
More issues are definitely needed but the Internet availability on my trip has been pretty bad. If someone could create more, I'd appreciate it, or I can create them when I get back on the 16th. |
No worries, I don't think it's urgent to happen before the 16th when you are back from AFK (and I didn't expect a response until then either, I should have clarified). Just wanted to understand what the intention and status was, is all. Thanks for the update and enjoy the rest of your time off! |
Fixes
Fixes #938 by @sarayourfriend
Description
This PR adds an implementation plan for blurring sensitive content in the frontend. You can read it here: https://docs.openverse.org/_preview/2118/projects/proposals/trust_and_safety/detecting_sensitive_textual_content/20230506-implementation_plan_frontend_blurring_sensitive.html
Checklist
Update index.md
).main
) or a parent feature branch.Developer Certificate of Origin
Developer Certificate of Origin