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

Request (more) correctly sized imgops preview image in Firefox #2400

Merged
merged 1 commit into from Jan 31, 2019

Conversation

paperboyo
Copy link
Contributor

@paperboyo paperboyo commented Jan 29, 2019

Because, unlike Chrome, Firefox differs screen.width with zoom, we need to treat it differently to correctly request screen-sized preview image.

Thanks @mbarton and @RichieAHB (who actually wrote all this).

  • test on TEST

@RichieAHB
Copy link
Contributor

RichieAHB commented Jan 29, 2019

Worth noting we wrote this in the GitHub editor so would be keen to make sure this was tested on CODE to make sure I didn't make any booboos.

Edit: I just saw @paperboyo 's checkbox 🙄

@RichieAHB
Copy link
Contributor

Additionally I think this will serve large images on retina displays in Firefox (devicePixelRatio is 2 on retina displays) - this might not be a problem but Chrome won't get this special treatment.

Copy link
Member

@akash1810 akash1810 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

once tested on TEST 👍

@paperboyo
Copy link
Contributor Author

@RichieAHB

this will serve large images on retina displays in Firefox

Shit! You’re right! I just checked on 27in Retina iMac and both browsers report 2560px screen.width but devicePixelRatio of 2 at 100% zoom and up it when zooming in. screen.width stays put in Chrome and differs on zoom in Firefox.

This is ridiculous! Forgetting performance considerations where we should maybe restrict image dimensions over certain limit and just wanting to serve 1:1 pixels to displays on different browsers – not sure what can we do apart from introducing even more absurd logic whereby if it’s ≥2 we would strip 2 and just take the rest?!

Thoughts?

@paperboyo
Copy link
Contributor Author

OK. Tested on TEST, on Retinas too. This improves behaviour in Firefox on all dprs. Doesn’t change behaviour in Chrome on all dprs. Makes the browsers behave the same on non-HiDPI displays. Makes browsers widely different on HiDPI displays (Firefox gets Retina-ready images, Chrome gets half the physical resolution).
Above is true on a Mac, PC may deal with HiDPI differently (and OS has a much more granular scaling controls).

Until browsers vendors/spec writers agree, not much we can do, unless I’m wrong ¯_(ツ)_/¯

@paperboyo paperboyo merged commit 0b55620 into master Jan 31, 2019
@paperboyo paperboyo deleted the mk-preview-image-FF-fix branch January 31, 2019 11:23
@paperboyo paperboyo changed the title Request correctly sized imgops preview image in Firefox Request (more) correctly sized imgops preview image in Firefox Jan 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants