-
Notifications
You must be signed in to change notification settings - Fork 24.4k
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
Whitelist the 'pdf' extension in the packager #7004
Conversation
By analyzing the blame information on this pull request, we identified @nicklockwood, @martinbigio and @amasad to be potential reviewers. |
@aprct updated the pull request. |
@nicklockwood Seems unrelated to anything in the pull request and that it's been happening to other people as well. |
Looks like the Travis CI machine's simulator is getting the Red Screen of Death :)
|
Tests seem to be fine on master. Can you rebase? |
@@ -83,7 +83,7 @@ function getPackagerServer(args, config) { | |||
'bmp', 'gif', 'jpg', 'jpeg', 'png', 'psd', 'svg', 'webp', // Image formats | |||
'm4v', 'mov', 'mp4', 'mpeg', 'mpg', 'webm', // Video formats | |||
'aac', 'aiff', 'caf', 'm4a', 'mp3', 'wav', // Audio formats | |||
'html', // Document formats | |||
'html', 'pdf', // Document formats |
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.
Sad that we have the same list in 3 places.
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.
Could we move this config to https://github.com/facebook/react-native/blob/master/packager/rn-cli.config.js? It would allow everyone to add the extensions they need.
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.
In that case would it have to be added to https://github.com/facebook/react-native/blob/master/packager/rn-cli.config.js as well? Still not exactly DRY if so.
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 pdf
is one of these things that should work as is in WebView (otherwise, except html
I don't see any other use-case for it in React Native)
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 don't think it should be in rn-cli.config
. It's not a configuration option. It's just the list of what's supported by RN. If someone adds a random extension there it won't work, since the library doesn't support it.
Summary: `RCTComponentData` needs `RCTConvert` class, but it doesn't import it directly. This diff is changing it. Should be noop. Reviewed By: fkgozali Differential Revision: D3185141 fb-gh-sync-id: f845e3555d30c9fd9b39579194590ddf103a5a19 fbshipit-source-id: f845e3555d30c9fd9b39579194590ddf103a5a19
Reviewed By: perryjrandall Differential Revision: D3154028 fb-gh-sync-id: a823fad9dd19af6ed4d585521ebcbaf79cdcc1e2 fbshipit-source-id: a823fad9dd19af6ed4d585521ebcbaf79cdcc1e2
Summary:The CSS spec doesn't allow for decimal values inside of rgb colors, however the RN implementation does, so there was a disconnect here. This tests to see if the output range is an rgb color, and if so, rounds the first 3 interpolated components (but not the 4th, since that would be opacity and allows for a decimal). cc vjeux Closes facebook#6984 Differential Revision: D3186473 fb-gh-sync-id: a320bf2311764e084386700bf8c8a42ab2a347eb fbshipit-source-id: a320bf2311764e084386700bf8c8a42ab2a347eb
…om/aprct/react-native into packager-support-for-pdf-extension
@aprct updated the pull request. |
@aprct updated the pull request. |
Ok we're all good! It's now as easy as: |
@facebook-github-bot shipit |
Thanks for importing. If you are an FB employee go to Phabricator to review. |
e9398c7
Summary:The WebView component in iOS currently does not support displaying PDFs without providing a remote URI or manually including the assets in the xcodeproj itself. This is because the packager has not whitelisted the 'pdf' extension. I've gone ahead and whitelisted the 'pdf extension according to the recommendation by nicklockwood GH comment: facebook#1846 (comment) Closes facebook#7004 Differential Revision: D3196019 Pulled By: nicklockwood fb-gh-sync-id: 10a86a9232095f98f277506141de0b8af5b21ab4 fbshipit-source-id: 10a86a9232095f98f277506141de0b8af5b21ab4
Summary:The WebView component in iOS currently does not support displaying PDFs without providing a remote URI or manually including the assets in the xcodeproj itself. This is because the packager has not whitelisted the 'pdf' extension. I've gone ahead and whitelisted the 'pdf extension according to the recommendation by nicklockwood GH comment: facebook#1846 (comment) Closes facebook#7004 Differential Revision: D3196019 Pulled By: nicklockwood fb-gh-sync-id: 10a86a9232095f98f277506141de0b8af5b21ab4 fbshipit-source-id: 10a86a9232095f98f277506141de0b8af5b21ab4
I want to show a PDF in a web view on android but it's coming up blanks. I've tried 2 url schemes Is it possible to display a PDF in an android web view? |
@npomfret If you're trying to display a PDF from an external URL, then give this a shot:
It won't work for Android yet. Google has a URL scheme to allow you to view a PDF through Google Drive, but it can look a little janky. To do that put this before your URL in the uri |
Summary:The WebView component in iOS currently does not support displaying PDFs without providing a remote URI or manually including the assets in the xcodeproj itself. This is because the packager has not whitelisted the 'pdf' extension. I've gone ahead and whitelisted the 'pdf extension according to the recommendation by nicklockwood GH comment: facebook#1846 (comment) Closes facebook#7004 Differential Revision: D3196019 Pulled By: nicklockwood fb-gh-sync-id: 10a86a9232095f98f277506141de0b8af5b21ab4 fbshipit-source-id: 10a86a9232095f98f277506141de0b8af5b21ab4
Summary:The WebView component in iOS currently does not support displaying PDFs without providing a remote URI or manually including the assets in the xcodeproj itself. This is because the packager has not whitelisted the 'pdf' extension. I've gone ahead and whitelisted the 'pdf extension according to the recommendation by nicklockwood GH comment: facebook/react-native#1846 (comment) Closes facebook/react-native#7004 Differential Revision: D3196019 Pulled By: nicklockwood fb-gh-sync-id: 10a86a9232095f98f277506141de0b8af5b21ab4 fbshipit-source-id: 10a86a9232095f98f277506141de0b8af5b21ab4
Summary:The WebView component in iOS currently does not support displaying PDFs without providing a remote URI or manually including the assets in the xcodeproj itself. This is because the packager has not whitelisted the 'pdf' extension. I've gone ahead and whitelisted the 'pdf extension according to the recommendation by nicklockwood GH comment: facebook/react-native#1846 (comment) Closes facebook/react-native#7004 Differential Revision: D3196019 Pulled By: nicklockwood fb-gh-sync-id: 10a86a9232095f98f277506141de0b8af5b21ab4 fbshipit-source-id: 10a86a9232095f98f277506141de0b8af5b21ab4
The WebView component in iOS currently does not support displaying PDFs without providing a remote URI or manually including the assets in the xcodeproj itself. This is because the packager has not whitelisted the 'pdf' extension.
I've gone ahead and whitelisted the 'pdf extension according to the recommendation by @nicklockwood
GH comment: #1846 (comment)