Page MenuHomePhabricator

Don't widen images when on metered connection
Closed, ResolvedPublic1 Story Points

Description

The app should not load higher resolution images when on a metered connection.

Event Timeline

bearND created this task.Feb 15 2016, 8:35 PM
bearND raised the priority of this task from to Needs Triage.
bearND updated the task description. (Show Details)
bearND moved this task to Needs Triage on the Wikipedia-Android-App-Backlog board.
bearND added a subscriber: bearND.
Restricted Application added subscribers: StudiesWorld, Aklapper. · View Herald TranscriptFeb 15 2016, 8:35 PM

Metered = cellular and not Wikipedia Zero.

Change 281950 had a related patch set uploaded (by Mholloway):
Don't widen images on metered connections

https://gerrit.wikimedia.org/r/281950

Mholloway claimed this task.Apr 6 2016, 3:19 PM
Mholloway moved this task from To Do to Code Review on the Mobile-App-Android-Sprint-79-Gold board.
Mholloway removed a subscriber: Mholloway.

Change 284751 had a related patch set uploaded (by Mholloway):
Add style directives to center un-widened images in Parsoid HTML

https://gerrit.wikimedia.org/r/284751

Mholloway added a comment.EditedApr 21 2016, 8:47 PM

The patch I created stops images from being widened on metered connections and ensures they're still centered properly when not widened, but there is actually more to this task when it comes to hi-res images. We have code in widenImages.js, mainly for the benefit of older phones/browsers, that parses out and grabs the highest-resolution images from any srcset attribute it finds when the image is being widened, since srcset was introduced in HTML5 and isn't universally "known" to browsers in the wild. So on older devices, the hi-res thumbnails won't be downloaded. Newer browsers, however (including, I think, the system webview on Android Lollipop+) opportunistically grab the "best" (usually highest-quality) image based on the srcset attribute without any developer intervention. On these platforms the app will grab a higher-res thumbnail even if image widening is disabled.

We no longer serve HiDPI thumbs on mobile web at all[1]; as a product question, is this a step we want to take in the Android app as well? Varying the presence of srcset attributes in the response based on the user's connection state would have negative response caching consequences that it's not obvious to me how we'd get around (unless we wanted to start having multiple endpoints to serve content with/without srcset attributes, which doesn't seem worth it). So I think we likely have to go one way or the other on this.

[1] T119797: Serve low-res images by default to users on slow or metered mobile connections

Change 284815 had a related patch set uploaded (by Mholloway):
Add styles for Parsoid images

https://gerrit.wikimedia.org/r/284815

Mholloway added a comment.EditedApr 22 2016, 1:12 AM

http://caniuse.com/#feat=srcset confirms that, beginning on Lollipop, the Android WebView understands srcset and therefore will automatically download larger/higher-quality images in the srcset attribute. I wonder whether it's worth preventing widening for metered connections only on KitKat and below, where the hi-res images aren't downloaded unless we explicitly do so with JavaScript in the course of widening. Since we're unlikely to want to do away with srcset/hi-res images/widening completely, that might be a good compromise.

Pinging @Nirzar for comment on this since it will have design implications.

Change 284751 merged by jenkins-bot:
Add style directives for images in Parsoid HTML

https://gerrit.wikimedia.org/r/284751

Change 284815 merged by jenkins-bot:
Add styles for Parsoid images

https://gerrit.wikimedia.org/r/284815

Change 281950 merged by jenkins-bot:
Don't widen images on metered connections

https://gerrit.wikimedia.org/r/281950

Dbrant added a subscriber: Dbrant.May 13 2016, 6:53 PM

QA: no functional changes expected, if using Wifi.

Testing on Android 5.1 ZTE phone and Wikipedia app 2.1.144-alpha-2016-05-13. This is fixed as there no functional changes during regression testing.

bearND closed this task as Resolved.May 18 2016, 5:31 PM
bearND moved this task from Ready for Signoff to Done on the Mobile-App-Android-Sprint-82-Lead board.
Mholloway set the point value for this task to 1.May 20 2016, 6:10 PM