Page MenuHomePhabricator
Create Task
Maniphest T328943

Replace PCS lazy-loading logic with standard "loading=lazy" attribute
Closed, ResolvedPublic
Actions

Assigned To
Dbrant
Authored By
Dbrant
Feb 6 2023, 5:39 PM
Tags
Referenced Files
F36921831: Screenshot 2023-03-21 at 9.41.34 AM
Mar 21 2023, 2:54 PM
F36921834: Screenshot 2023-03-21 at 9.35.28 AM
Mar 21 2023, 2:54 PM
F36921826: Screenshot 2023-03-21 at 9.30.29 AM
Mar 21 2023, 2:54 PM
F36921823: Screenshot 2023-03-21 at 9.38.26 AM
Mar 21 2023, 2:54 PM
Subscribers
Aklapper
Dbrant
Jgiannelos
MSantos
TheDJ
Tsevener
vadim-kovalenko

Description

We have some fairly heavy and sophisticated code and transformations in PCS to enable lazy-loading of images, when in fact the HTML standard has had a simple loading="lazy" attribute for some time now, which is supported in the latest WebViews of both Android and iOS platforms.

In the Android case, the System WebView is updated pretty aggressively and automatically (for obvious security reasons), so the vast majority of Android users will have the latest Chromium-based WebView (109).

In the iOS case, it looks like global usage of WebViews older than 15.4 accounts for ~2% of users.[1] Perhaps @Tsevener can fill in any other risks here, but it seems at first glance like there aren't other major road blocks.

[1] https://caniuse.com/loading-lazy-attr

Details

Related Changes in Gerrit:
SubjectRepoBranchLines +/-
Update and modernize image lazy-loading, take 2.mediawiki/services/mobileappsmaster+78 -750
Remove lazy-loading logic and replace with loading=lazy attribute.mediawiki/services/mobileappsmaster+16 -844
Customize query in gerrit

Related Objects
Search...

StatusSubtypeAssignedTask
 ResolvedDbrantT328943 Replace PCS lazy-loading logic with standard "loading=lazy" attribute
 ResolvedMSantosT314025 [EPIC] Migrate PCS service away from restbase
 ResolvedJgiannelosT314764 Branch out mobileapps for restbase deprecation
 ResolvedJgiannelosT314765 Replace RESTBase endpoint references of outgoing API calls
 ResolvedJgiannelosT314766 Use core page HTML as a replacement for RESTBase parsoid calls
ResolvedJgiannelosT314767 Log outgoing RESTBase API calls
 ResolvedJgiannelosT314768 [spike] Performance benchmarking of mobileapps without RESTBase
 ResolvedJgiannelosT314770 [spike] Performance benchmarking of RESTBase/mobileapps without cassandra
 ResolvedJgiannelosT323514 Disable storage for testwiki PCS on restbase production
 ResolvedJgiannelosT329837 Find a wiki to disable cassandra storage for `mobile-html` endpoint
 ResolvedJgiannelosT314781 Perfomance baseline metrics for mobileapps
 ResolvedJgiannelosT314782 Create a dataset of wiki pages to run as a test/benchmark suite
 ResolvedMSantosT314771 Implement a compatibility layer between RESTBase and native PCS responses
 ResolvedMSantosT321194 Implement Security Response Header Filter logic outside RESTBase for PCS
 ResolvedJgiannelosT321195 Implement Access Check Filter outside RESTBase for PCS
 OpenNoneT321199 Implement Ensure Content Type Filter logic outside RESTBase for PCS
 ResolvedJgiannelosT314777 Investigate cache purging needs
 ResolvedJgiannelosT321101 Investigate moving summary endpoint outside of mobileapps service
 ResolvedJgiannelosT264670 Move PCS endpoints behind API Gateway
 ResolvedhnowlanT372746 hewiki: Route mobile-html to the backing node service instead of RESTBase
 ResolvedJgiannelosT372749 hewiki: Use backing node service instead of RESTBase on pregeneration changeprop rules
 ResolvedJgiannelosT385719 testwiki: Add changeprop rule to send requests to PCS on resource change
 ResolvedJgiannelosT385821 Dont propagate server error details to end users
 ResolvedJgiannelosT387277 Rollout more wikis after week 1 of testing with production traffic
 ResolvedJgiannelosT388214 Pregeneration rules don't pregenerate caches for the same cases restbase did
 ResolvedJgiannelosT388140 Rollout more wikis: week 2
 ResolvedJgiannelosT385033 Switchover plan from restbase to rest gateway for PCS endpoints that don't require cache
 ResolvedWRai-WMFT389073 PCS switchover: test editing on testwiki and/or User namespace
 ResolvedJgiannelosT390080 Accept language is not working for srwiki
 ResolvedJgiannelosT390724 Rollout more wikis: week 3
 ResolvedTsevenerT389074 [Testing] PCS switchover: test editing on testwiki and/or User namespace
 ResolvedJgiannelosT392052 PCS caching metrics: Add label to distinct production vs pregeneration traffic
 OpenBUG REPORTNoneT363279 /page/summary/ and /page/mobile-html/ should handle BCP47 languge code for wikidata description
 ResolvedMSantosT392491 Page talk endpoint is not used by any of the apps
 ResolvedJgiannelosT393591 Rollout more wikis: week 4
 OpenNoneT321207 Core REST endpoints should cover needs of node services
 ResolvedMSantosT301372 Core Page REST API should follow redirects
 ResolvedTK-999T290639 Move redirect lookup logic into a service object
 DuplicateNoneT321190 page/{title}/HTML endpoint should support "virtual" content on message pages
 Resolvedabi_T317019 REST page/html endpoints should support language variants
 DuplicateNoneT262593 MediaWiki Developer stores different HTML output for different language variants in the parser cache
 Resolvedabi_T318401 Language conversion in page/html endpoints need to fall back to old LanguageConverter classes of Parsoid doesn't support the conversion.
 OpenNoneT322426 Create REST endpoint for getting the latest "stable" HTML
OpenNoneT326742 REST API: Wiki redirects include an HTML representation of the redirect in the body of the response
 OpenNoneT326743 REST API: include a machine readable representation of redirect targets in responses
 InvalidNoneT322152 Route external requests to RESTbase via API gateway
 ResolvedJgiannelosT333343 Cache control header configuration on restbase is not working for PCS endpoints
 OpenNoneT338126 Spike: Investigate LanguageConverter redirects for language variants
 InvalidJgiannelosT340129 PCS performance optimization to avoid the need for pregeneration
 ResolvedJgiannelosT343120 Investigate cache needs based on existing logs/metrics
 ResolvedJgiannelosT344943 Determine how PCS will be informed about resource changes in a world without RESTbase
 ResolvedJgiannelosT319365 PCS caching and pregeneration when restbase is decommissioned
 ResolvedcoolteyT322142 Measure network latency of page load requests (Android)
 ResolvedSNowick_WMFT327548 Report network latency of page load requests (Android)
 ResolvedSNowick_WMFT332769 pt.wiki latency P95 + P99 in apps pre- and post-disabling of pregeneration in PCS
 ResolvedEevansT348993 Create new cassandra table data model for PCS
 ResolvedJgiannelosT348995 Introduce PCS cache management layer
ResolvedhnowlanT350507 Update mobileapps k8s deployment chart for Cassandra credentials
 ResolvedJgiannelosT348996 Change changeprops rules to pre-generate/invalidate cache directly to PCS rather than in restbase
 ResolvedJgiannelosT366819 Enable PCS to send resource change events to handle URL purges
 ResolvedJgiannelosT368052 Allow connections from PCS to eventgate
 ResolvedJgiannelosT385706 Move cache control headers emitted by restbase to PCS
 ResolvedJgiannelosT385718 PCS raises errors when trying to send requests to eventgate

Event Timeline

Dbrant created this task.Feb 6 2023, 5:39 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 6 2023, 5:39 PM
gerritbot added a comment.Feb 6 2023, 6:48 PM

Change 886932 had a related patch set uploaded (by Dbrant; author: Dbrant):

[mediawiki/services/mobileapps@master] [WIP] Remove lazy-loading logic and replace with loading=lazy attribute.

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

gerritbot added a project: Patch-For-Review.Feb 6 2023, 6:48 PM
LGoto triaged this task as Low priority.Feb 7 2023, 7:24 PM
LGoto added a project: iOS Release FY2023-24 (Archive).
LGoto moved this task from Needs Triage to Engineering Backlog on the Wikipedia-iOS-App-Backlog board.
Jgiannelos added a project: Content-Transform-Team-WIP.Feb 8 2023, 3:33 PM
Jgiannelos moved this task from Backlog to Code Review on the Content-Transform-Team-WIP board.
Jgiannelos subscribed.Feb 13 2023, 3:59 PM

This ticket looks related: https://phabricator.wikimedia.org/T325623
I would prefer to have a more systematic approach like the one in the WIP patch from @Dbrant rather than manually improving lazy loading.
@Tsevener is this something we can do in iOS too?

vadim-kovalenko subscribed.Feb 13 2023, 4:28 PM
• JMcLeod_WMF assigned this task to Dbrant.Feb 13 2023, 4:34 PM
• JMcLeod_WMF moved this task from Code Review to Blocked on the Content-Transform-Team-WIP board.
Tsevener added a comment.Feb 13 2023, 7:02 PM

@Jgiannelos Agreed. I would need to do some testing to see how it acts once the patch is ready. It seems like Safari on iOS does support it by default, for iOS15+, if I'm reading https://caniuse.com/loading-lazy-attr correctly. We do support iOS14 as well, so we'll need pull some numbers on iOS14 usage and test to see if the degradation is acceptable for those users.

LGoto moved this task from Tasks from Product Backlog to Blocked or Waiting on the iOS Release FY2023-24 (Archive) board.Feb 14 2023, 7:34 PM
Dbrant mentioned this in T325623: Bug: Images don’t appear until scrolling down.Mar 21 2023, 1:36 PM
Tsevener added a comment.EditedMar 21 2023, 2:54 PM

The best I could test is via different versions of Simulator. Here are some screenshots from Safari inspector. The steps I took for this are:

  1. Load Elizabeth II.
  2. Clear the session cache.
  3. Go to simulator settings, clear the Safari cache.
  4. In Safari on machine, connect a web debug session to simulator article view.
  5. Pull to refresh on article in simulator.

Pre-change, iOS 14:

Screenshot 2023-03-21 at 9.38.26 AM (1×1 px, 289 KB)

Post-change, iOS 14 (loading spinner was shown during the localhost call, ~2.5 seconds). Subsequent attempts were in the 1.5~2s range.

Screenshot 2023-03-21 at 9.30.29 AM (1×3 px, 982 KB)

iOS 15 & 16 had about the same performance post-change as iOS 14 pre-change:

Post-change, iOS 15:

Screenshot 2023-03-21 at 9.35.28 AM (923×1 px, 259 KB)

Post-change, iOS 16:

Screenshot 2023-03-21 at 9.41.34 AM (1×1 px, 289 KB)

Tsevener added a comment.Mar 21 2023, 4:02 PM

Offline mode still works for me across all OSes. We can also guide iOS users to enabling lazy image loading in iOS Settings > Safari > Advanced > Experimental Features > Lazy image loading.

Dbrant moved this task from Blocked to Code Review on the Content-Transform-Team-WIP board.Mar 21 2023, 4:14 PM
Tsevener moved this task from Blocked or Waiting to Needs Code Review on the iOS Release FY2023-24 (Archive) board.Mar 22 2023, 1:26 PM
Tsevener moved this task from Needs Code Review to Blocked or Waiting on the iOS Release FY2023-24 (Archive) board.Mar 24 2023, 1:20 PM
dr0ptp4kt moved this task from Code Review to Blocked on the Content-Transform-Team-WIP board.Apr 3 2023, 3:20 PM
MSantos moved this task from Blocked to Code Review on the Content-Transform-Team-WIP board.Apr 5 2023, 2:19 PM
MSantos subscribed.

During the Apps Sync, we have talked about how minimal the impact will be so it's safe to move forward.

gerritbot added a comment.Apr 7 2023, 5:36 PM

Change 886932 merged by jenkins-bot:

[mediawiki/services/mobileapps@master] Remove lazy-loading logic and replace with loading=lazy attribute.

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

Maintenance_bot removed a project: Patch-For-Review.Apr 7 2023, 6:10 PM
MSantos moved this task from Code Review to To Deploy on the Content-Transform-Team-WIP board.Apr 10 2023, 3:08 PM
Jgiannelos mentioned this in T334827: Images missing in Android and iOS applications.Apr 17 2023, 11:37 AM
dr0ptp4kt moved this task from To Deploy to In Progress on the Content-Transform-Team-WIP board.Apr 17 2023, 3:14 PM
SLopes-WMF moved this task from In Progress to Blocked on the Content-Transform-Team-WIP board.May 30 2023, 2:15 PM
MSantos added a project: Essential-Work.Jun 19 2023, 11:27 AM
TheDJ subscribed.Nov 21 2023, 3:27 PM

Can this be closed ?????

Jgiannelos added a comment.Nov 21 2023, 3:34 PM

No, this patch got eventually reverted because it caused an outage (mismatch between js/html version of pcs output)
https://gerrit.wikimedia.org/r/c/mediawiki/services/mobileapps/+/908961

TheDJ added a comment.Nov 21 2023, 4:02 PM

Related ticket to that revert T334827: Images missing in Android and iOS applications

TheDJ added a comment.Nov 21 2023, 7:32 PM
In T328943#9349643, @Jgiannelos wrote:

No, this patch got eventually reverted because it caused an outage (mismatch between js/html version of pcs output)
https://gerrit.wikimedia.org/r/c/mediawiki/services/mobileapps/+/908961

So if i understand it correctly, if the transform is rolled out first, and then after some while the JS change is made again, this should be fixable right ?

Jgiannelos added a comment.Nov 22 2023, 3:01 PM

No because the JS loaded doesn't necessarily apply to the right document because the page/mobile-html output might be older (due to pregeneration).
In order for this rollout to work we need to have a way to request a specific version of our JS assets that its not easy at the moment with pcs being served on RESTBase.
If that's not possible we need to force a new output pregeneration for each new response but this is going to need a lot of time.

I think that currently we are prioritizing moving PCS away from RESTBase and when this happens we will have more flexibility on how to serve versioned CSS/JS assets.

JTannerWMF removed a project: iOS Release FY2023-24 (Archive).Jan 23 2024, 10:49 PM
JTannerWMF moved this task from Tech Debt Backlog to Tracking on the Wikipedia-Android-App-Backlog board.
TheDJ changed the task status from Open to Stalled.Jan 24 2024, 8:40 AM
TheDJ added a subtask: T314025: [EPIC] Migrate PCS service away from restbase.
TheDJ mentioned this in T230812: Use native image lazy loading on mobile site in Parsoid.Aug 21 2024, 11:54 PM
gerritbot added a comment.Oct 23 2024, 5:51 PM

Change #1082528 had a related patch set uploaded (by Dbrant; author: Dbrant):

[mediawiki/services/mobileapps@master] [Super WIP] Update and modernize image lazy-loading, take 2.

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

gerritbot added a project: Patch-For-Review.Oct 23 2024, 5:51 PM
Dbrant changed the task status from Stalled to In Progress.Mar 17 2025, 1:08 PM
gerritbot added a comment.Apr 3 2025, 5:04 PM

Change #1082528 merged by Cooltey:

[mediawiki/services/mobileapps@master] Update and modernize image lazy-loading, take 2.

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

Maintenance_bot removed a project: Patch-For-Review.Apr 3 2025, 5:31 PM
Jgiannelos edited projects, added Content-Transform-Team (Work In Progress); removed Content-Transform-Team-WIP.Apr 9 2025, 12:29 PM
Jgiannelos moved this task from Backlog to To Deploy on the Content-Transform-Team (Work In Progress) board.
TheDJ awarded a token.Apr 9 2025, 2:31 PM
Jgiannelos moved this task from To Deploy to To Verify on the Content-Transform-Team (Work In Progress) board.May 5 2025, 3:08 PM
Jgiannelos closed this task as Resolved.May 6 2025, 2:06 PM
MSantos closed subtask T314025: [EPIC] Migrate PCS service away from restbase as Resolved.Sep 4 2025, 2:29 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits