Skip to content
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

Default viewer doesn't use 'retina' toolbar icons on iOS #6685

Closed
jasongisstl opened this issue Nov 24, 2015 · 14 comments
Closed

Default viewer doesn't use 'retina' toolbar icons on iOS #6685

jasongisstl opened this issue Nov 24, 2015 · 14 comments

Comments

@jasongisstl
Copy link

When using the default viewer provided with PDF.js on iOS (retina), the toolbar icons appear blurry.

This seems to be because the following media query in viewer.css isn't being matched:
@media screen and (min-resolution: 2dppx)

Changing this to include a webkit-specific prefix seems to solve this:
@media screen and (min-resolution: 2dppx), (-webkit-min-device-pixel-ratio: 2)
(My CSS knowledge isn't great, so apologies if there are problems with this)

I realise that users are expected to build upon the default viewer, but it would be nice if this worked out of the box.

(Tested using iOS 9.1 Safari & UIWebview)

@yurydelendik
Copy link
Contributor

Safari on desktop affected as well, the following shall be fine for our preprocessor:

@media
  screen and (-webkit-min-device-pixel-ratio: 2),
  screen and (min-resolution: 2dppx) {

@prohtex
Copy link

prohtex commented Jan 22, 2018

I'm wondering if any progress has been made on this one. I am implementing pdfjs for clients who exclusively use tablets and mobile devices, and many comment that the icons are "blurry." On a desktop with a high-resolution display, the issue is not as critical as the icons are much smaller relative to the view area. But on a mobile device, it is odd.

img_0829

@yurydelendik
Copy link
Contributor

yurydelendik commented Jan 22, 2018

I'm wondering if any progress has been made on this one.

Nope. Any ideas why safari is doing it?

@prohtex
Copy link

prohtex commented Jan 22, 2018

Isn't the issue that no one has created high-resolution assets to be served to the browser? I'm also wondering why the desktop viewer always displays a sans-serif font while iOS uses what looks like Times Roman!

@yurydelendik
Copy link
Contributor

yurydelendik commented Jan 22, 2018

Isn't the issue that no one has created high-resolution assets to be served to the browser?

Are these need to be in Safari specific format? WFM on Firefox and Chrome. Also text looks good as well.

@prohtex
Copy link

prohtex commented Jan 22, 2018

I'm using Safari mobile on iOS devices. I don't have access to an android phone rn to check, but I assumed the assets simply weren't large enough to display properly.

@timvandermeij
Copy link
Contributor

timvandermeij commented Jan 23, 2018

Every image has an @2x variant for hi-dpi screens (see https://github.com/mozilla/pdf.js/tree/master/web/images) and this is served when the media query matches, so from our perspective this should be working (and has been tested on actual hi-dpi screens). Most likely Safari mobile on iOS is doing something strange that causes the media query not to match.

@prohtex
Copy link

prohtex commented Jan 23, 2018

Thank you @timvandermeij for this information. I'm not sure how to troubleshoot this, but can report that I've viewed many builds of pdfjs over the past 3 years on a broad array of iOS devices -- iPhones and iPads of all stripes. In every case, the wrong assets were served, and the wrong font was used.

@timvandermeij
Copy link
Contributor

It looks like the problem is that Safari only supports the webkit prefixed versions of the device pixel media queries. Referring to https://caniuse.com/#feat=css-media-resolution, it looks like Safari is the only one that requires a prefixed media query for this, so that's rather unfortunate. However, the solution mentioned in the original message above would most likely solve this. It would be good if you could test if adding -webkit-min-device-pixel-ratio: 2 solves the issue. If so, we could find a way to incorporate it in PDF.js.

@Snuffleupagus
Copy link
Collaborator

Snuffleupagus commented Jan 23, 2018

I'm not sure how to troubleshoot this,

Did you try the suggestion from #6685 (comment), since that seems like a good starting point?

@prohtex
Copy link

prohtex commented Feb 3, 2018

I can confirm that the solution suggested does work, on Safari and iOS. All that needs to be changed is this line in viewer.css:

@media screen and (min-resolution: 2dppx) {

to

@media
	screen and (-webkit-min-device-pixel-ratio: 2),
	screen and (min-resolution: 2dppx) {

This can be seen working here:

http://www.thing.gallery/test/pdf.js/web/viewer.html

Now if we can figure out #9392, pdfjs will start looking a lot better on iPhones and iPads.

@prohtex
Copy link

prohtex commented Feb 14, 2018

@timvandermeij, it seems the solution I have posted above completely resolves this issue. I am not exactly sure how to create a PR for this, but was wondering if one of the devs might be able to create the PR given it is only 2 lines of code. I tried just now but I don't think I have the permissions.

@darrenahunter
Copy link

this is a big +1 from me too
thanks

@timvandermeij
Copy link
Contributor

Closing since this is fixed by #9629.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants