Page MenuHomePhabricator

Parsoid doesn't set image width/height if they were missing in the original wikitext
Open, MediumPublic


@Jdforrester-WMF reported this as a bug with upright support, but I think the bug is more general. It just so happens that [[File:Foo.jpg|thumb|upright=0.5]] is a special case of this:

$ echo '[[File:Foobar.jpg]]' | tests/parse.js --prefix enwiki | sed -e 's/height="28" width="240"/height="200" width="200"/' | tests/parse.js --prefix enwiki --html2wt

$ echo '[[File:Foobar.jpg|thumb|upright=1]]' | tests/parse.js --prefix enwiki | sed -e 's/height="26" width="220"/height="200" width="200"/' | tests/parse.js --prefix enwiki --html2wt

But this works:

$ echo '[[File:Foobar.jpg|240px]]' | tests/parse.js --prefix enwiki | sed -e 's/height="28" width="240"/height="200" width="200"/' | tests/parse.js --prefix enwiki --html2wt

Event Timeline

cscott raised the priority of this task from to Needs Triage.
cscott updated the task description. (Show Details)
cscott added a project: Parsoid.
cscott added subscribers: cscott, Jdforrester-WMF.

Hm, there's an explicit comment in the parserTests that change in size is ignored so long as class='mw-default-size'. The mw-default-size class indicates that the image is scaled by the default thumbnail size, so changes to the user's default thumbnail size should be applied to this image. This is correct for upright.

The issue then seems to be primarily on the VE side: it should clear mw-default-size when an image size is modified.

Alternatively, Parsoid shouldn't *ignore* changes to size when mw-default-size is present; instead it should use them to emit an appropriate upright flag (or scale or square flag in the future) to scale the default thumb size. For @T64671 purposes, VE can present a checkbox for "scale the default thumbnail size" which adds/removes mw-default-size, rather than directly exposing the (deprecated, misnamed) upright flag.

Interesting historical note: one of the reasons we originally didn't implement the above ("use mw-default-size as an indicator to scale the default size") is that it would require an async imageinfo query during wikitext serialization (WTS), and currently our WTS code is all synchronous. With the addition of data-file-width and data-file-height attributes in the Parsoid output (T64881) we can not implement this synchronously *iff* the user provides us with the data-file-width and data-file-height attributes. This means that, for instance, if VE inserts a new image it must be sure to *also* add data-file-* attributes for the new image, or else the mw-default-size button (the UI for T64671, whatever it is called) won't work.

Or we could make our whole WTS process async.

Continuing the discussion -- the reason why I need to know the original image dimensions is because VE always gives me a square bounding box (one dimension, ve-size), but upright always scales on width. So I need to use the original image dimensions to determine if the image is portrait or not. If it's landscape (height < width), then upright-ratio = ve-size / default-thumb-width, but if it's portrait, then actual-width = (ve-size / original-height) * original-width and then upright-ratio = actual-width / default-thumb-width.

Hence the original T351: RfC: Square bounding boxes -- if we had an option *like* upright, but which scaled a square bounding box which was default-thumb-width on each side -- let's call it scale for purposes of discussion---then I could simply emit scale=<ratio> where <ratio> = ve-size / default-thumb-width without having to consult the original image's aspect ratio.

Change 238855 had a related patch set uploaded (by Cscott):
Allow serialization of images which scale the default thumbnail size.

ssastry triaged this task as Medium priority.Sep 16 2015, 10:52 PM
ssastry set Security to None.