Page MenuHomePhabricator

Add image - image insertion issue
Closed, ResolvedPublic

Description

per @MMiller_WMF feedback in "Add an image" testing

When I got to the caption step on this article, the image preview element turned this strange color. The validation warning’s icon appeared on top of the message. And the visual treatment for a Visual Editor “table” appeared around the image preview.

Screen Shot 2021-11-16 at 11.53.52 AM.png (1×850 px, 820 KB)

And even after I saved the image (see second screenshot), the strange color remained in the article itself.

Screen Shot 2021-11-16 at 11.54.01 AM.png (1×936 px, 936 KB)

@Tgr This is probably a bug in the inserting logic, it puts the image inside the first non-template element with editable contents. I assumed that would be a paragraph, but apparently table cells match the condition as well.

Event Timeline

I think this has to do with how ve.dm.ElementLinearData.isContentOffset() works. The article in case was Bayern-Kaserne@510852 (Parsoid HTML), with content like

<section> <table> <tbody> <tr> <th>...</th>...
         ^       ^       ^    ^     ^
         1       2       3    4     5

1, 2, 3 and 4 are what VE calls structural offsets - you can't insert text there, you need to insert some sort of structure first (in the case of 1 a <p> tag, for 2/3/4 a table caption / row / cell).. 5 is a content offset: you can add text and inline content such as an image. I assumed content offset always means <p> and so that's always the right place for an image (and hence the insertion logic finds the first such offset that's not between templates), but apparently table cells are content offsets as well.

I wonder if I got the assumption wrong? We are inserting an MWBlockImageNode, not an MWInlineImageNode, so maybe it should actually go to the structural offset instead? Or maybe it doesn't really matter and VE can figure out either way? Probably worth consulting the Editing team on this.

@MMiller_WMF we'd want to insert on top of the table, right? (Both table and image are right-floated so one will end up above the other.)

I filed another insertion breakage issue - T295922: [testwiki-wmf.9] Add image - small size image breaks caption box. I think that the issue happens because of a small size of the image, but I might be wrong about.

@MMiller_WMF we'd want to insert on top of the table, right? (Both table and image are right-floated so one will end up above the other.)

@Tgr -- yes, we want the image to be above the table -- the spirit is that it should be the first thing in the article after the templates at the top (same as specifications here T290785: Add an image: edit business rules.

Change 740294 had a related patch set uploaded (by Gergő Tisza; author: Gergő Tisza):

[mediawiki/extensions/GrowthExperiments@master] [WIP] Add Image: Do not insert inside tables

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

Change 740294 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] Modify the insertion logic to insert before, not at, the first non-whitespace content offset, as that could be inside a table, list etc.

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

Tested on testwiki wmf.12 with the same article the issue was reported - Bayern-Kaserne.

The insertion of the image (and displaying the caption validation error) is fine:

Screen Shot 2021-12-08 at 5.59.06 PM.png (720×415 px, 121 KB)
Screen Shot 2021-12-08 at 5.59.26 PM.png (708×401 px, 245 KB)
Screen Shot 2021-12-08 at 5.59.42 PM.png (691×393 px, 227 KB)
Screen Shot 2021-12-08 at 6.00.26 PM.png (574×385 px, 149 KB)

Note (not relevant to the ticket's scope): the position of the table in the article looks odd to begin with:

Screen Shot 2021-12-08 at 6.02.24 PM.png (395×1 px, 115 KB)

Accordingly, after adding an image, the position of the added image is not optimal:
Screen Shot 2021-12-08 at 6.00.49 PM.png (570×1 px, 289 KB)

Accordingly, after adding an image, the position of the added image is not optimal:

Screen Shot 2021-12-08 at 6.00.49 PM.png (570×1 px, 289 KB)

That's the coordinate template being broken. Not much we can do about it & probably won't happen on real wikis.