Last modified: 2014-08-28 19:39:35 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T68610, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 66610 - VisualEditor: Image CE HTML should match Parsoid
VisualEditor: Image CE HTML should match Parsoid
Status: RESOLVED FIXED
Product: VisualEditor
Classification: Unclassified
Technical Debt (Other open bugs)
unspecified
All All
: High enhancement
: VE-deploy-2014-08-28
Assigned To: James Forrester
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2014-06-14 09:04 UTC by Ed Sanders
Modified: 2014-08-28 19:39 UTC (History)
3 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description Ed Sanders 2014-06-14 09:04:28 UTC
When converting between block and inline images we hide the <figcaption> with CSS instead of removing it from the DOM tree. We need to preserve the caption in case the user converts from inline back to block, but this can be done in the model.
Comment 1 Ed Sanders 2014-06-14 09:05:38 UTC
Discussion from Gerrit:

Mooeypoo		Jun 13 8:25 PM

<figcaption> isn't destroyed and/or is recreated each time an image type changes, so it has to be hidden for the block image types that don't support it, like frameless and basic.
But yes, it could be added to the CSS instead, hiding the figcaption for the block image types that don't support it. 
I'm not sure it should be generalized for Parsoid's use, though, since the Parsoid output for the figcaption is always correct (so, no caption for images that don't support it). We keep the caption in VE to let the user be able to change the images back and forth and retain the information in the caption even if the type changed.

Esanders		Jun 13 8:29 PM

If it's not supposed to be in the tree when it's not in use, why don't we destroy it completely and recreate it when it's needed again?

Mooeypoo		Jun 13 8:40 PM

Because the caption information is taken from <figcaption> and we want to retain it for (A) if the user regrets their type choice and edits again to another type that supports the caption (make sure that the caption isn't lost) and (B) so it won't be removed from the wikitext.
There's a tech debt to fix the way caption works in general, especially in light of VE allowing the switch from block to inline and the existence of alternate text (which gets the value of the caption if it exists) but right now it's on hold for a little bit waiting for the new Media Edit dialog redesign.

Esanders		Jun 13 8:43 PM

If we need to preserve the caption that should be done in the model, not the view.

Mooeypoo		Jun 13 8:48 PM

Yes, and it also must be preserved in wikitext, so the <figcaption> must be sent to parsoid in certain cases. And the caption surface in the editor should be selectively disabled and enabled, and exchange its information with the alt-text in a way that the user understands where and why things are replaced.
Hence the tech debt.

GWicke		Jun 13 9:36 PM

Also see https://gerrit.wikimedia.org/r/#/c/139473/. There should be no need to manually hide the caption with inline CSS. Inline images have no caption in the DOM, as that can for example break a surrounding paragraph in case the caption contains a paragraph.
Comment 2 James Forrester 2014-08-21 23:08:59 UTC
What's left to do here?
Comment 3 James Forrester 2014-08-28 19:39:35 UTC
Current Parsoid HTML:

<figure class="mw-default-size" typeof="mw:Image/Thumb" data-parsoid="…">
	<a href="./File:Smokeycover.jpg" data-parsoid="…">
		<img resource="./File:Smokeycover.jpg" src="//upload.wikimedia.org/wikipedia/en/thumb/7/73/Smokeycover.jpg/170px-Smokeycover.jpg" height="237" width="170" data-parsoid="…">
	</a>
	<figcaption data-parsoid="…">
		<a rel="mw:WikiLink" href="./Smokey_Stover" title="Smokey Stover" data-parsoid="…">Smokey Stover</a><!-- no italics, character’s name --> driving a "foomobile", with <i data-parsoid="…">phoo</i> visible on the front
	</figcaption>
</figure>


Current VE HTML:

<figure class="ve-ce-mwBlockImageNode ve-ce-mwBlockImageNode-type-thumb mw-default-size ve-ce-branchNode ve-ce-mwBlockImageNode-borderwrap mw-halign-right ve-ce-focusableNode" typeof="mw:Image/Thumb" contenteditable="false" style="width: 172px; height: auto;">
	<a class="image" href="https://en.wikipedia.org/wiki/File:Smokeycover.jpg">
		<img src="//upload.wikimedia.org/wikipedia/en/thumb/7/73/Smokeycover.jpg/170px-Smokeycover.jpg" class="ve-ce-mwBlockImageNode-thumbimage" style="width: 170px; height: 237px;">
	</a>
	<figcaption class="ve-ce-branchNode">
		<div class="magnify"><a class="internal"></a></div>
		<p class="ve-ce-paragraphNode ve-ce-generated-wrapper ve-ce-branchNode">
			<a class="ve-ce-linkAnnotation ve-ce-mwInternalLinkAnnotation new" href="http://en.wikipedia.beta.wmflabs.org/wiki/Smokey%20Stover" title="Smokey Stover">Smokey Stover</a><span class="ve-ce-leafNode ve-ce-commentNode oo-ui-indicatedElement-indicator oo-ui-indicator-alert oo-ui-indicatedElement ve-ce-focusableNode" contenteditable="false"> </span>&nbsp;driving a "foomobile", with <i class="ve-ce-textStyleAnnotation ve-ce-italicAnnotation">phoo</i> visible on the front
		</p>
	</figcaption>
</figure>

The only structural difference is the "<div class="magnify"><a class="internal"></a></div>" which Parsoid is missing (and will go away soon enough anyway). Deeming this to be fixed.

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links