Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Remove contributors file from NVDA, read license as HTML #17600
base: master
Are you sure you want to change the base?
Remove contributors file from NVDA, read license as HTML #17600
Changes from all commits
6bff723
ff3576f
60c0a72
6e035d9
30ab607
97ec694
0215d3f
d12a774
bdbcff0
e40448b
79be781
3f2f2f1
04f2242
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
I understand this comment, it's however not clear from the comment how activating links is avoided here. I assume that links are not rendered in browseable messages?
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.
it's avoided by the blocking the whole action with the blockAction. Links are rendered in the browseable messages. I'm not sure if I can place the comment any closer to the decorator while keeping formatting happy. Maybe
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.
Ah I see now, thanks for clarifying.
Can't we instruct nh3 to strip links in a secure context, for example by providing an empty list of url schemes?
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.
We could, but I think it's useful for these links to work, no?
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.
It definitely is 😉
However, we can make the behavior of nh3 differ from the default behavior when in a secure context, for example:
In secure context, we probably also want to pass something like url_schemes=set() to ensure that external comments will never load.
The latter might even be enough, as long as we don't provide something like
<a href="something">something</a>
as these won't be filtered.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.
@SaschaCowley - do you think this is necessary?
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.
Potentially useful, but not necessary. Note this would also require changes to browsable message, which is currently not available when running on a secure desktop.
nvda/source/ui.py
Lines 156 to 158 in fa619c2
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.
I think your first suggestion (putting the explanatory comment in the decorator) is the way to go here, @seanbudd