Page MenuHomePhabricator

Remove mw:Placeholder and data-parsoid from mw:DisplaySpace
Closed, ResolvedPublic

Description

From looking at VE, it doesn't look like the mw:Placeholder is doing anything:

modules/ve-mw/dm/nodes/ve.dm.MWEntityNode.js:

ve.dm.MWEntityNode.static.matchRdfaTypes = [ 'mw:Entity', 'mw:DisplaySpace' ];
ve.dm.MWEntityNode.static.allowedRdfaTypes = [ 'mw:Placeholder' ];

Semantically, mw:Placeholder is supposed to mean, "don't touch this". That's not the case here, mw:DisplaySpace should be treated just the same as any other space.

Note that treating mw:DisplaySpace the same as an explicit entity is probably not *quite* right, as there may be corner cases where the context around the mw:DisplaySpace changes how it is serialized. In the worse case this could lead to either a dirty diff (where a plain space in the original turns into a non-breaking space) or a violation of WYSIWYG (where a non-breaking space in VE turns into a plain space in the original). It's not entirely clear what the "right" behavior here is; given that VE treating mw:DisplaySpace as mw:Entity has not led to complaints so far, we can probably keep this as-is. But there's the possibility that we might want to tweak this behavior somehow in the future.

As T254512: mw:DisplaySpace doesn't need a data-parsoid src points out, once we remove mw:Placeholder we can/should also remove the data-parsoid attribute on these spans.

Event Timeline

Change 602437 had a related patch set uploaded (by C. Scott Ananian; owner: C. Scott Ananian):
[mediawiki/extensions/VisualEditor@master] French spacing (mw:DisplaySpace) doesn't have mw:Placeholder any more

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

ssastry triaged this task as Medium priority.Jun 4 2020, 8:06 PM
cscott renamed this task from Remove mw:Placeholder from mw:DisplaySpace to Remove mw:Placeholder and data-parsoid from mw:DisplaySpace.Jun 4 2020, 8:21 PM
cscott updated the task description. (Show Details)

Depending on the resolution here, we might need to mark mw:DisplaySpace as uneditable in the TestRunner.
See https://gerrit.wikimedia.org/r/c/mediawiki/services/parsoid/ /602477

Update the spec when this is resolved.

Change 615296 had a related patch set uploaded (by Arlolra; owner: Arlolra):
[mediawiki/services/parsoid@master] Remove mw:Placeholder from mw:DisplaySpace

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

Change 602437 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] French spacing (mw:DisplaySpace) doesn't have mw:Placeholder any more

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

Change 615296 merged by jenkins-bot:
[mediawiki/services/parsoid@master] Remove mw:Placeholder from mw:DisplaySpace

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

Arlolra claimed this task.

Change 628944 had a related patch set uploaded (by Subramanya Sastry; owner: Subramanya Sastry):
[mediawiki/vendor@master] Bump wikimedia/parsoid to 0.13.0-a10

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

Change 628944 merged by jenkins-bot:
[mediawiki/vendor@master] Bump wikimedia/parsoid to 0.13.0-a10

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

Change 629176 had a related patch set uploaded (by Arlolra; owner: Arlolra):
[mediawiki/services/parsoid@master] Avoid modified-wrapper on mw:DisplaySpace

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

Change 629176 merged by jenkins-bot:
[mediawiki/services/parsoid@master] Avoid modified-wrapper on mw:DisplaySpace

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

Change 630676 had a related patch set uploaded (by Subramanya Sastry; owner: Subramanya Sastry):
[mediawiki/vendor@master] Bump wikimedia\parsoid to 0.13.0-a11

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

Change 630676 merged by jenkins-bot:
[mediawiki/vendor@master] Bump wikimedia/parsoid to 0.13.0-a11

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

To follow-up, VisualEditor finally removed its hack to ignore mw:Placeholder on mw:DisplaySpace while the cache cycled.