-
Notifications
You must be signed in to change notification settings - Fork 10.1k
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
Load file:-URLs in the main thread. #5100
Conversation
@timvandermeij Could you take a look at this PR? |
I think this looks good, but I'll let someone else make the call for this one. One comment though: can we rename |
I've renamed |
I guess we can take it. I think we need to minimize commented code for linting. Can you move xhr logic into e.g. It makes sense to have |
@yurydelendik I have removed the comments to allow linting. This snippet will be included in the GENERIC build only, which is the default, so it makes sense to have the code uncommented by default. I haven't added |
// very quickly, so there is no need to set up progress event listeners. | ||
PDFView.setTitleUsingUrl(file); | ||
var xhr = new XMLHttpRequest(); | ||
xhr.responseType = 'arraybuffer'; |
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 you move this line after xhr.open to avoid exceptions in Firefox?
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.
Done.
QtWebKit does not support file:-URLs in Web Workers. To solve this issue, I'm loading file:-URLs in the main thread for generic builds. file:-URLs load relatively quick, so there is no need for sophisticated load progress notifications.
Load file:-URLs in the main thread.
Thanks |
QtWebKit does not support file:-URLs in Web Workers.
To solve this issue, I'm loading file:-URLs in the main thread for generic builds. file:-URLs load relatively quick, so there is no need for sophisticated load progress notifications.
Fixes #5057