Any updates?

Change 517179 had a related patch set uploaded (by Aklapper; owner: Aklapper):
[mediawiki/extensions/timeline@master] Add image dimensions (width, height) to HTML output

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

Not sure why this was triaged as "Normal" priority by Readers-Web if it's only under "Tracking".

Patch linked above is entirely untested and might be utterly wrong. Note that EasyTimeline is unmaintained deprecated software, see T137291: Replace all uses of EasyTimeline and decommission it from Wikimedia's servers.

Also see T216431: Graphs create img tags without width and height about the same issue with the Graph extension.

According to https://www.mediawiki.org/wiki/Developers/Maintainers , the code stewards of EasyTimeline are the WMF Editing Team hence adding Editing-team to get code review for the patch.

Ping - would the Editing Team please review this patch? Thanks.

Rather than the getimagesize( $url ) in above patch, a better code could be:

$png = $backend->getFileContents( [ 'src' => "{$fname}.png" ] );
getimagesizefromstring( $png );

Additionally, when the PNG has been newly generated in the request, we could even avoid the file read, by reusing $svgWidth/$svgHeight from the rasterization code.

@Od1n: Please feel very welcome to amend the patch in Gerrit! Thanks :)

Using the Gerrit Patch Uploader, even after proper rebasing of my patch, I'm encountering this error:

! [remote rejected] HEAD -> refs/for/master (cannot add patch set to 517179.)

Apparently it's a permission issue?

Change 690771 had a related patch set uploaded (by Gerrit Patch Uploader; author: Jean Ducont):

[mediawiki/extensions/timeline@master] Add image dimensions (width, height) to HTML output

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

After numerous tries, it was a permission issue indeed.
Uploaded to a separate changeset, so you can at least see my patch: 690771.

There are several ways to write this code, I picked the one I think is the clearer.
It should be quite robust, because if the PNG has been successfully created, we can assume its width and height are retrievable.

Also, when the PNG has been newly generated in the request, I really wanted to avoid reading it back just to get the dimensions we already know.

Rather than reading the entire PNG to get its dimensions, maybe we could store them in a sidecar file? (something like {$pathPrefix}.dimensions)

Change 517179 abandoned by Aklapper:

[mediawiki/extensions/timeline@master] Add image dimensions (width, height) to HTML output

Reason:

Superseded by https://gerrit.wikimedia.org/r/c/mediawiki/extensions/timeline/ /690771/

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

Thanks for the patch. I guess the problem now will be to find someone to review, as EasyTimeline is unmaintained...

I understand this code better now (didn't notice there are 2 ways of generating the PNG: default using ploticus only, and "svg2png").

Thus I improved the patch, to get the dimensions the more efficiently possible in each situation:

• When generating new files, in "svg2png" mode, reuse $svgWidth and $svgHeight variables (ensuring they have been filled, otherwise fallback to next point).
• When generating new files, in default mode, get the dimensions from the temporary local PNG file.
• When files were already generated, get the dimensions from the PNG file stored in backend.

I also tried to implement a sidecar ".dimensions" file, but it's significantly more complicated, mainly because the files never expire actually, so there are many code branches to tweak (for timelines already existing and for the ones created after the patch).

Ping - could WMF's Editing-team please review Od1n's 20 line patch in https://gerrit.wikimedia.org/r/c/mediawiki/extensions/timeline/+/690771/ so readers can also see timelines on mobile? Thanks.

@VPuffetMichel: Ping - could WMF's Editing-team please review Od1n's 20 line patch in https://gerrit.wikimedia.org/r/c/mediawiki/extensions/timeline/+/690771/ so readers can also see timelines on mobile? Thanks!

@Aklapper Thanks for sending this our way. It seems that the editing team has never maintained easy timeline and it is currently not owned by anyone on https://www.mediawiki.org/wiki/Developers/Maintainers

I am new and I am curious why you thought of us for a code review. (just trying to piece things together)

@VPuffetMichel: Sorry, looks like I missed this change when the Editing Team removed themselves as stewards. I'll file another code stewardship request...

I'm already touching this code for T289226: Convert EasyTimeline extension to use Shellbox so I left a review on the patch and can help shepherd it.

I have seen your comment on Gerrit. Indeed, it was a shame to fetch the PNG every time from backend to get its dimensions. As I stated above, I previously made an attempt at implementing a sidecar metadata file, but it got too much complicated, mainly because of the diversity of code flows. Though, I just did another attempt with fresh mind (and meanwhile discovered the "create" backend operation instead of "store", which helped to keep the code simple)…

Therefore, I worked on and released a new patch (see changeset 5), which implements the desired metadata JSON file :)

Reviews are welcome. There probably should be refinements to make (code comments, required safety checks…)

Uploaded a new patch (see changeset 6), which avoids errors if ever PHP failed to determine the dimensions of the PNG, or if the desired data couldn't be read in the JSON retrieved from backend.

Merge conflict with this large change. I'm not planning to rebase my patch…