fix(mac): allow display of package help file linked content 🍎 - #16458
fix(mac): allow display of package help file linked content 🍎#16458sgschantz wants to merge 2 commits into
Conversation
.pdf, .txt and .png files will now be loaded locally when linked from help files in package Fixes: #9628
keymanapp-test-bot
Bot
added
has-user-test
user-test-required
User Test Results |
keymanapp-test-bot
Bot
changed the title
Test Specs
Test Results
|
keymanapp-test-bot
Bot
removed
the
user-test-required
| private static let allowedLocalExtensions = ["pdf", "txt", "png", "jpg", "jpeg"] | ||
| private static let allowedMimeTypes = ["application/pdf", "text/plain", "image/jpeg", "image/png"] |
There was a problem hiding this comment.
I am uncomfortable with this. There may be many other file types included, e.g. webp, gif, videos, css, fonts. This is hard to whitelist correctly.
Is this for navigation operations only?
There was a problem hiding this comment.
Yes, it's only for navigation to load the other files that are located in the same directory. I need to force load them because there is a security layer in the webview that prevents them from loading otherwise. I've changed the logic to check for the link location instead, but not sure if it handles all cases.
| if let mimeType = navigationResponse.response.mimeType { | ||
| if PackageContentWebView.allowedMimeTypes.contains(mimeType) { | ||
| decisionHandler(.allow) | ||
| return | ||
| } | ||
| } | ||
| decisionHandler(.allow) |
There was a problem hiding this comment.
This seems to be a nothing test? both code paths give decisionHandler(.allow)
There was a problem hiding this comment.
Yes, that's useless, and I can still access the content when I don't implement this callback. I'll delete it.
instead check whether content is in the directory with the rest of the package
mcdurdin
left a comment
mcdurdin
left a comment
There was a problem hiding this comment.
LGTM - logic looks good; we definitely want to block navigation to files outside the package folder.
4 participants
.pdf, .txt and .png files are loaded locally when linked from help files in package
Fixes: #9628
Build-bot: release:mac
User Testing
Run Keyman Configuration and install the Syriac/Aramaic keyboard
Select the ? icon to open the help for the package
Click on the 'Aramaic (Hebrew layout)' link to load that child page
Click on both 'Font license' text link and the 'Font documentation' PDF link to verify that they load correctly