Page MenuHomePhabricator

Make available original image dimensions info from the API in Parsoid DOM.
Closed, ResolvedPublic0 Story Points

Description

Since Parsoid already runs an API call for each image given to it in wikitext, it would be extremely helpful for the editing in terms of user experience if the call also asked for the original dimensions.

At the moment, VisualEditor asks for details from the API on each edited image to get its original dimensions. When this is done for the edit dialog the asynchronous call works fine because we don't mind letting the user wait the seconds it takes the async call to finish.

But when the user resizes images with the mouse (with the resize handles) the information about original dimensions usually comes too late. The user may (and often does) resize the image to above its limits, and by the time VE gets the API response, the user is already done resizing. On top of that, calculating aspect ratio from original size is a lot more accurate than calculating it from the given (usually smaller) size, though, granted, this is a lot less crucial.

We have some fall backs and we are considering an after-the-fact user experience ideas to make this work, but it would be extremely helpful if the information was already available when it comes from Parsoid -- especially since Parsoid already asks for information from the API.


Version: unspecified
Severity: enhancement

Details

Reference
bz62881

Event Timeline

bzimport raised the priority of this task from to Needs Triage.Nov 22 2014, 3:06 AM
bzimport added a project: Parsoid.
bzimport set Reference to bz62881.
Mooeypoo created this task.Mar 20 2014, 6:15 PM

I'm not convinced we should be bloating our Parsoid DOM representation with original size information which can easily be obtained from the imageinfo API. In order to do a proper resize, you are probably going to have to fetch the original image anyway so that things don't get blurry when you scale up.

But I'm interesting in hearing other thoughts.

(In reply to C. Scott Ananian from comment #1)

I'm not convinced we should be bloating our Parsoid DOM representation with
original size information which can easily be obtained from the imageinfo
API. In order to do a proper resize, you are probably going to have to
fetch the original image anyway so that things don't get blurry when you
scale up.
But I'm interesting in hearing other thoughts.

We don't fetch the original image unless the new size is the image's original dimensions – we ask MW for the thumbnail of that size. We certainly don't ask e.g. for a 10MiB original 5000x4000px image when the user is just resizing from 250x200px to 300x240px. Also, we fetch the new image /after/ the image is resized.

I do agree we should be careful, but I think this is worth including. It's really useful and costs very little.

We have a new strategy for serializing square bounding boxes that will be helped by having original image size hanging around, so we will probably put original image size in data-mw.

On top of the dimensions, it would also be helpful to get the 'mediatype' property of images, separating them between "DRAWING" and "BITMAP". The two types behave differently, especially with size considerations.

It is useful for VE to know (we request this now with our separate api call) but it might actually make sense to categorize the difference between the images for other purposes.

The size concern also matters less once we move data-mw out of the DOM. There's still the minor nuisance of having some read-only attributes in data-mw, which is otherwise a read-write interface.

Maybe it would make sense to group dimensions & image type in a separate 'imageinfo' sub-object inside data-mw that is documented to be read-only. The grouping & naming should make it clearer that this is read-only info only, even without looking at the docs.

Arlolra triaged this task as Medium priority.Nov 26 2014, 10:35 PM
Arlolra added a subscriber: Arlolra.

Per conversations last week this should be reprioritised.

Esanders set Security to None.
Restricted Application added a project: VisualEditor. · View Herald TranscriptFeb 6 2015, 7:46 PM
Jdforrester-WMF moved this task from Backlog to VE Q3 on the Parsoid board.Feb 9 2015, 9:47 PM

Mediawiki core seems to use data-file-width and data-file-height on <img> tags for this. I wonder when that was added to core?

That comes from Multimedia Viewer or some such.

What do we want to happen if the target image doesn't exist? Presumably leave off the attributes, right?

Change 195666 had a related patch set uploaded (by Cscott):
T64881: Add original dimension information for images.

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

Uploaded a patch ^ which uses data-file-width and data-file-height to match the PHP HTML output created by the MediaViewer extension.

Perhaps this bloats the DOM, and we should use data-mw instead. I don't have a strong opinion here. My hand-wavy argument is that data-mw should be used for properties which are important/signficiant for editors in terms of defining a particular wikitext serialization. These new data-file-* properties are not consulted for WTS, they are just ignored.

(But perhaps we should consult these in order to serialize upright images correctly?)

matmarex removed a subscriber: matmarex.Mar 10 2015, 8:58 PM

Change 195798 had a related patch set uploaded (by Cscott):
T64881: Add original dimension information for images to data-mw.

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

Summary of IRC conversation:

So, if we had to add this info, it makes sense to add data-file-width and data-file-height attributes because (a) it is read only (b) mediaviewer and other gadgets already expect it and when Parsoid html is used for read views, those applications don't break suddenly (c) most pages have few images relative to the size of the DOM itself, so the DOM bloat is expected to be minor.

Parsoid is probably already handling HTML invalidation (via parsoid cache update jobs, to be confirmed for sure) and so there are no new cache invalidation issues that crop up from this change.

Restating subbu's (b) slightly, for more accuracy: Mediaviewer already adds it (to PHP parser output), and so other gadgets might expect it.

Should missing/error images have data-file-width/data-file-height attributes? Right now I'm not adding them.

Change 195798 abandoned by Cscott:
T64881: Add original dimension information for images to data-mw.

Reason:
Abandoned in favor of https://gerrit.wikimedia.org/r/195666

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

Change 195666 merged by jenkins-bot:
T64881: Add original dimension information for images.

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

This will go out on Monday. Roan confirmed that there are no VE changes that are required. Also for a while, VE should be prepared to accept HTML with / without these attributes till all the revisions turn over in RESTBase.

ssastry renamed this task from Add original dimensions information from the API to data-mw to Make available original image dimensions info from the API in Parsoid DOM..Mar 26 2015, 11:57 PM
ssastry closed this task as Resolved.
ssastry removed a project: Patch-For-Review.
Jdforrester-WMF moved this task from Blocked to Q4 on the VisualEditor board.Mar 29 2015, 10:47 PM

Is there a task for VE to use this now?

cscott added a comment.Apr 3 2015, 4:29 PM

@Esanders, @Jdforrester-WMF: perhaps you VE guys should add one? I think @Mooeypoo was doing the original work here (a year ago!).

Chiming in from iOS, we'd love to know the current thumbnail and original size so we can handle scaling up to higher resolutions elegantly. Is it worth making which properties to include configurable? Maybe some clients don't want it, maybe some do?

Disregard, this was already closed