Maniphest T220668

Regression: Download icon doesn't download images
Closed, ResolvedPublic2 Estimated Story Points
Actions

Assigned To

Authored By

	Jdlrobson
	Apr 11 2019, 12:14 AM

Description

While testing a patch from @Niedzielski I noticed that clicking the download icon doesn't lazy load images before the print action.

QA steps

Make sure "expand all sections" is disabled in Special:MobileOptions
Visit https://en.m.wikipedia.beta.wmflabs.org/wiki/Barack_Obama when anonymous (log out)
Use mobile site on a mobile resolution (desktop mode seems to be working fine) with sections collapsed by default
Click the print button (depicted with a red outline below) in the toolbar and print to pdf

Expected: PDF has images
Actual: PDF does not have images

Developer notes

The JS is working perfectly. The issue is the CSS animation. It prints in the starting position which is fade in. Resetting the animation in print @media solves this problem.

QA Results

AC	Status	Details
1	✅	T220668#5359958

Details

	Subject	Repo	Branch	Lines +/-
	Make sure lazy loaded images display in printed PDFs	mediawiki/skins/MinervaNeue	master	+6 -0

Customize query in gerrit

Related Objects

Mentioned In: rSKINff40b6baed71: Update git submodules
Mentioned Here: T214658: Separate Lazy loading code from Skin.js

Event Timeline

Jdlrobson created this task.Apr 11 2019, 12:14 AM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptApr 11 2019, 12:14 AM

Jdlrobson renamed this task from Download icon doesn't download images to Regression: Download icon doesn't download images.Apr 11 2019, 12:15 AM

Jdlrobson added projects: Regression, MinervaNeue.

Jdlrobson updated the task description. (Show Details)

I tried to repro this on https://en.m.wikipedia.org/wiki/Barack_Obama but it seems to be working.

I tried a number of other pages including local development articles using the content proxy but they seem to be working too. I'm thinking that the most likely issue is either that you're hitting the parsimonious timeout of 3 seconds or an image is failing to download (errors are silently eaten). You may be able to verify the latter by examining the network requests issues after pressing the download PDF button.

Thanks for the sanity check stephen! I will take another look tomorrow. Hopefully it's the timeout like you say (which seems quite low on retrospect for an entire article!)

Hmm.
So I can replicate this on https://en.m.wikipedia.beta.wmflabs.org/wiki/Qatar and https://he.m.wikipedia.org/wiki/%D7%94%D7%9E%D7%A2%D7%A8%D7%9B%D7%94_%D7%A2%D7%9C_%D7%A1%D7%99%D7%A0%D7%99_%D7%95%D7%90%D7%A8%D7%A5_%D7%99%D7%A9%D7%A8%D7%90%D7%9C_%D7%91%D7%9E%D7%9C%D7%97%D7%9E%D7%AA_%D7%94%D7%A2%D7%95%D7%9C%D7%9D_%D7%94%D7%A8%D7%90%D7%A9%D7%95%D7%A0%D7%94 and https://en.m.wikipedia.beta.wmflabs.org/wiki/Albert_Einstein

When I test on English Wikipedia e.g. https://en.m.wikipedia.org/wiki/Qatar it's fine.

This means this is likely a new bug that is going to hit production. What could have changed recently that impacted that?

Firstly, can you replicate with these new instructions?

Not sure what the heck I'm doing wrong but https://en.m.wikipedia.beta.wmflabs.org/wiki/Qatar shows the Wikipedia:New user landing page when I'm logged in and the Qatar page when I'm logged out:

en.m.wikipedia.beta.wmflabs.org_w_index.php_title=Wikipedia_New_user_landing_page&page=Qatar.png (1×1 px, 308 KB)

Regardless, I cannot repro on any of the pages you mentioned :| What are you seeing in the console?

Jdlrobson updated the task description. (Show Details)Apr 11 2019, 5:05 PM

Jdlrobson updated the task description. (Show Details)

I can repro on emulated Android with sections collapsed on enwiki prod.

Jdlrobson updated the task description. (Show Details)Apr 11 2019, 5:12 PM

ovasileva triaged this task as Medium priority.Apr 16 2019, 10:20 AM

Jdlrobson removed • Niedzielski as the assignee of this task.Jul 9 2019, 5:51 PM

Jdlrobson updated the task description. (Show Details)

Change 521551 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@master] Make sure lazy loaded images display in printed PDFs

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

gerritbot added a project: Patch-For-Review.Jul 9 2019, 5:51 PM

Jdlrobson moved this task from Needs Prioritization to Triaged but Future on the Web-Team-Backlog board.Jul 9 2019, 5:51 PM

Jdlrobson moved this task from Triaged but Future to Upcoming on the Web-Team-Backlog board.Jul 16 2019, 4:49 PM

ovasileva edited projects, added Web-Team-Backlog (Readers-Web-Kanbanana-2019-20-Q1); removed Web-Team-Backlog.Jul 16 2019, 5:08 PM

ovasileva moved this task from Needs Analysis to Needs Code Review on the Web-Team-Backlog (Readers-Web-Kanbanana-2019-20-Q1) board.

ovasileva set the point value for this task to 2.Jul 16 2019, 5:13 PM

• nray assigned this task to Edtadros.Jul 16 2019, 11:19 PM

• nray moved this task from Needs Code Review to Needs QA on the Web-Team-Backlog (Readers-Web-Kanbanana-2019-20-Q1) board.

Change 521551 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Make sure lazy loaded images display in printed PDFs

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

Restricted Repository Identity mentioned this in rSKINff40b6baed71: Update git submodules.Jul 16 2019, 11:40 PM

ReleaseTaggerBot added a project: MW-1.34-notes (1.34.0-wmf.15; 2019-07-23).Jul 17 2019, 12:00 AM

Maintenance_bot removed a project: Patch-For-Review.Jul 17 2019, 12:12 AM

Test Result

Status: ❌ FAIL
OS: macOS Mojave
Browser: Chrome
Device: MBP
Emulated Device: iPhoneX

Test Artifact(s):

QA steps

Make sure "expand all sections" is disabled in Special:MobileOptions
Visit https://en.m.wikipedia.beta.wmflabs.org/wiki/Qatar
Use mobile site on a mobile resolution with sections collapsed by default
Click the print button and print to pdf

❌ AC1: Expected: PDF has images

T220668-AMC-ON.pdf1 MBDownload

T220668-AMC-OFF.pdf1 MBDownload

@nray Just to be sure, there is no print button, I used the print button from the browser.

Edtadros updated the task description. (Show Details)Jul 18 2019, 2:33 AM

@Edtadros sorry, I needed to update the QA instructions. You should be able to see the print button if you follow the instructions now

• nray reassigned this task from • nray to Edtadros.Jul 18 2019, 3:59 PM

• nray moved this task from Needs More Work to Needs QA on the Web-Team-Backlog (Readers-Web-Kanbanana-2019-20-Q1) board.

Test Result

Status: ✅ PASS
OS: macOS Mojave
Browser: Chrome
Device: Google Pixel 3 XL

Barack Obama - Wikipedia, the free encyclopedia.pdf3 MBDownload

Test Artifact(s):

QA steps

Make sure "expand all sections" is disabled in Special:MobileOptions
Visit https://en.m.wikipedia.beta.wmflabs.org/wiki/Barack_Obama when anonymous (log out)
Use mobile site on a mobile resolution (desktop mode seems to be working fine) with sections collapsed by default
Click the print button (depicted with a red outline below) in the toolbar and print to pdf

✅ AC1: Expected: PDF has images

@nray I executed this on BrowserStack so that I could use an android device. It took some acrobatics to get that file!

Edtadros updated the task description. (Show Details)Jul 24 2019, 4:54 AM

all done here, thanks all!

	F29859048: Barack Obama - Wikipedia, the free encyclopedia.pdf
	Jul 24 2019, 4:53 AM

	F29800895: Screen Shot 2019-07-18 at 9.54.59 AM.png
	Jul 18 2019, 3:57 PM

	F28617742: en.m.wikipedia.beta.wmflabs.org_w_index.php_title=Wikipedia_New_user_landing_page&page=Qatar.png
	Apr 11 2019, 4:58 PM

	F29798010: T220668-AMC-ON.pdf
	Jul 18 2019, 2:31 AM

	F29798009: T220668-AMC-OFF.pdf
	Jul 18 2019, 2:31 AM

Regression: Download icon doesn't download imagesClosed, ResolvedPublic2 Estimated Story PointsActions

Description

QA steps

Developer notes

QA Results

Details

Related Objects

Event Timeline

Test Result

QA steps

Test Result

QA steps

QA steps

Regression: Download icon doesn't download images
Closed, ResolvedPublic2 Estimated Story Points
Actions