Twenty Seventeen: iPad mini: Images on the front page badly pixelated

[alex27]

On iPad mini 2 retina in landscape mode both custom header image and panel images appear to be enlarged and badly pixelated:

In portrait mode images display correctly:

Original size of featured image above is 2400x1600.

Featured images on single posts/pages look good as well.

[karmatosed]

Discussion over here: ​https://github.com/WordPress/twentyseventeen/issues/450 - ticket being brought over as part of merge.

[laurelfulford]

background-attachment: fixed is causing this weird issue.

In 38395.patch​, I added a check to see if the theme is being loaded on an iOS device, and used that to change a CSS class. The applied CSS class is then used to change the background-attachment styles.

[davidakennedy]

Nice work around here @laurelfulford! I'd like to avoid user agent sniffing. It's a slippery slope.

It seems this is disabled in some mobile browsers for performance reasons. See: ​https://twitter.com/paul_irish/status/306818591196602368

I'm wondering if there's another solution? Maybe with the pseudo element, using it to be the thing that's fixed?

[laurelfulford]

Increasing screen size where background-attachment fixed is applied.

[laurelfulford]

This is trickier to fix than I expected!

38395.1.patch​ isn't ideal, but it increases the breakpoint where the background-attachment: fixed is applied to the front page images. Doing this removes the fixed backgrounds and the issue from all iOS devices, but the downside is that it also removes the fixed backgrounds from smaller laptops.

[LittleBigThing]

Hi,

Raising the min-width is indeed necessary, but it needs to be relatively high to cover the 12.9 inch iPad Pro. This mean that a lot of users with narrowor desktops will experience the theme without the fixed background image.

I was thinking of having an additional CSS rule that is specific for iOS devices or at least Apple devices. I could only think of the resolution (in dpi).

Couldn't we throw in an additional test for the media feature resolution? This is simple CSS but it could narrow down the use of background-attachment: scroll...

Something like:

@media only screen and (max-width: 85.45em) and ( (resolution: 264dpi) or (resolution: 326) or (resolution: 401) ) {
    .panel-image {
        background-attachment: scroll; 
    }
}

I am not sure how well this CSS feature is supported and if it needs the min or max prefix. ​It seems that we could also use only the prefixed (-webkit-) version and maybe do something with the non-standard rule min/max-device-pixel-ratio (?).
But I could be totally wrong, sorry... (did not have the time to test it yet).

Cheers,
Csaba

[laurelfulford]

Thanks for the suggestion, @LittleBigThing!

I tested adding a resolution to the media query, and the one downside I found is that it's still not possible to distinguish between iOS devices and retina screens that way. So it makes it possible to keep the fixed backgrounds on some smaller laptops, but not all.

I did a bit more searching in this vein, and found a post about targeting iPad Pros using really specific device height and width queries: ​https://medium.com/connect-the-dots/css-media-queries-for-ipad-pro-8cad10e17106

In 38395.2.patch​, I lowered the screen size where background-attachment: fixed is applied, and added an iPad Pro-sized media query to override it. This exact setup still feels a bit hacky to me, but less heavy handed than the user agent sniffing. Suggestions for improvements welcome!

[davidakennedy]

We talked about this ticket in the bug scrub today and in Core Themes in Slack. @laurelfulford and I decided to go with 38395.1.patch​ because it's the simplest and less targeted. Meaning, it's remotely possible this could be changed in the future should the landscape change.

See: ​https://wordpress.slack.com/archives/core-themes/p1478651088000264

[davidakennedy]

In 39176:

Twenty Seventeen: Fix badly pixelated images on iOS devices

The CSS property background-attachment: fixed is disabled in some mobile browsers for performance reasons. So here, the fix increases the breakpoint where the background-attachment: fixed is applied to the front page images. Doing this removes the fixed backgrounds and the issue from all smaller screens, but the downside is that it also removes the fixed backgrounds from smaller laptops. It's also possible future devices could fall into this breakpoint, but this seems to be the most practical solution.

Props laurelfulford.

Fixes #38395.

[helen]

The fixed background effect is lost for a lot of normal desktop sizes (1367px and under), which is a real shame because it's one of the things that's really initially attractive about Twenty Seventeen. Since the committed approach is already a workaround, it probably makes sense to dial in the media queries to be really specific about devices targeted (with explanatory inline comments, naturally).

[LittleBigThing]

Hi,
Maybe give it a try combining it with pixel density (dpi) based media queries? (comment #7)
Macbooks' and iOS devices' screens have different screen pixel densities (​https://www.tekrevue.com/retina-display-comparison/).
Greetings,
Csaba

[karmatosed]

It would be great to get a more targeted patch - is anyone able to rustle that up? If not, I can investigate tomorrow a little more myself before RC. I agree with @helen that we should avoid losing the effect on so many desktops.

[joemcgill]

I'm not sure if feature detection will work here, but 38395.3.patch​ adds a test to see if background-attachment: fixed is supported before setting the style.

[joemcgill]

Was able to do some testing of 38395.3.patch​ on an iOS simulator, and it looks like this type of feature detection won't work because Safari still reports that 'background-attachment: fixed' is supported, even though it's being disabled under the hood. I'm usually against UA sniffing, but in this case, I think adding UA sniffing like what is in 38395.patch​ is preferable over trying to use a media query as a proxy for filtering out a specific browser/OS issue.

[joemcgill]

38395.4.patch​ sticks with the feature test approach from 38395.3.patch​ but includes the checkiOS() test from 38395.patch​ when determining support for fixed backgrounds. I also returned the media query for applying this style to min-width: 55em which was the value before the [39176] workaround.

[celloexpressions]

UA sniffing seems like the clear choice here since we can't reliably do feature detection, and the alternative is a major design regression for a much broader set of devices.

[davidakennedy]

Thanks everyone for the continued work and thoughts on this. I'm glad to have another chance to make this better.

@joemcgill I tested the patch heavily in current desktop browsers, plus devices in Browserstack. It works well overall, and I think this approach is better than just adjusting media queries. It doesn't make sense to punish other devices that may support this feature.

One thing I did notice: in testing in Browserstack on a Nexus 7 tablet with Chrome 49 – the function returned true, which I wouldn't expect it would. Is that what false reporting the Stack Overflow issue talked about?

Even so, I'm still up for going this route since it seems like the best option.

[joemcgill]

Replying to davidakennedy:

One thing I did notice: in testing in Browserstack on a Nexus 7 tablet with Chrome 49 – the function returned true, which I wouldn't expect it would. Is that what false reporting the Stack Overflow issue talked about?

I'm not sure if the Nexus 7 issue you're seeing is specific to Browserstack or not since I don't have a device to test on, but I'm seeing the same behavior in Browserstack. I'm not sure if it's because the media query isn't applying or if there's something else going on with the test, but I'm not seeing any visual problems with the behavior.

If we do find that there are other browser quirks that need to be addressed, those could be handled much like the UA check for iOS devices within supportsFixedBackground().

[helen]

I tested the patch + trunk + trunk before the original commit on both a Nexus 7 and an iPad Mini (in real life), patch works correctly for me in practical user terms. My only question left is if the media query needs to stay so wide at 55em given feature detection, and if there should be a comment there indicating the reasoning behind whatever value that becomes.

[laurelfulford]

In 38395.5.patch​ I updated the patch to change the breakpoint where background-attachment: fixed is applied from 55em to 48em. That's the same breakpoint where the panel image is set to be 100% of the screen height, along with most of the other 'large screen' styles. It seemed like a natural fit for the effect to me, but it could go lower if that makes sense.

Also included a comment explaining the current location :)

[davidakennedy]

In 39297:

Twenty Seventeen: Adds background-attachment: fixed; to devices that should support it

iOS disables this feature under the hood, but it also distorts the images – unlike other mobile devices that don't support it. So this adds a check for both background-attachment: fixed support or whether it’s an iOS device - passing it adds the class background-fixed which is used to add the proper styles.

It also lowers the media query so the parallax-like style is present on a wider range of screens since this bug can be better targeted and avoided. In this way, screens that aren't the offending devices aren't punished merely based on screen size.

Props joemcgill, laurelfulford, helen.

Fixes #38395.

[mapk]

I still get the distorted image on iOS devices here: ​https://make.wordpress.org/core/features/ when I preview the theme on my iOS device.

[domainsupport]

Just found this old thread. I've posted a fix here https://core.trac.wordpress.org/ticket/52797

Oliver