Page MenuHomePhabricator

Value-less extension attributes not preserved
Open, MediumPublic

Description

<syntaxhighlight strict lang="php">$foo</syntaxhighlight>

round trips to

<syntaxhighlight strict="" lang="php">$foo</syntaxhighlight>

Either Parsoid needs to fix this, or we need to tell extension authors that attributes must have values (as per XML).

Event Timeline

Esanders raised the priority of this task from to Needs Triage.
Esanders updated the task description. (Show Details)
Esanders added a project: Parsoid.
Esanders subscribed.
ssastry triaged this task as Medium priority.Jun 9 2015, 3:23 PM
ssastry set Security to None.

Implementing this would be a little problematic for VE too, as we use a XML builder to generate the wikitext for updating the rendering. We'd probably have to switch to sending over the Parsoid HTML and having the API round-trip it to update the rendering.

A few questions come to mind:

  • Does the dirtying affect the semantics of the tag extension? Or is it just a noisy dirty diff?
  • If the latter, and if only a handful of extensions are affected, we could make a case for tag extensions to be xml-compliant and then use a bot to fix up.
  • If semantics are affected and/or a lot of extensions use these kind of properties, it will take more work.

I suspect not many extensions use this, and even fewer would have a flag attribute that could also be a value attribute. That said, <syntaxhighlight> claimed in documentation that using 'line' without a value was equivalent to the default value, line="FACNY_LIES", although this was all lies and the php only cared about the presence of the attribute (see https://www.mediawiki.org/w/index.php?title=Extension%3ASyntaxHighlight_GeSHi&type=revision&diff=1673241&oldid=1659681 ).

Without doing a full audit of extensions I couldn't say for sure if there are any other instances.

I think a dirty diff of attr to attr="" will annoy users though, so if we do go down the XML route, we should have a default value for booleans, e.g. ="1"

The problem with requiring a value is that ="" looks messy in the source and="1" may confuse users into thinking that ="0" will disable the feature, while many extensions are coded to just check for attribute existence.

I think HTML5 says that an empty attribute is actually itself, not the empty string? (Let me check...)

https://stackoverflow.com/questions/21796217/are-attributes-without-value-allowed-in-html4

Alas, "empty attribute syntax" is one of the things which changed between XML, HTML4, SGML, and HTML5. And wikitext extension tags are not any of those, really, so of course we don't have to match them exactly. But it would be nice to move to the HTML5 semantics, just to reduce surprise.

Parsoid should probably just serialize empty attribute values using the empty attribute syntax, since that's almost always what's wanted. That would take care of most of the dirty diff issue. Of course, we could/should tweak the PHP side API to use the HTML5 semantics (that is, treat empty attribute syntax and the empty string value as identical, as far as extension code can tell).

Please provide me with an update? Are you working on it?

@cscott Are you still working on this bug and VisualEditor changes of <references responsive> to <references responsive =""> ?

Is someone still working on this? I imagine, that this should be a very small hack, or am I wrong?

@Esanders : What do you think, is it a Parsoid bug or Parsoid serializing bug, that edits by VisualEditor change automatically the source code

I am not very sure, but it looks like a problem with this commit, doesn't it? https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Cite/+/560850

https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Cite/+/560850/3/src/Cite.php#496 Line 496 is now:
$responsive !== null ? $responsive !== '0' : $wgCiteResponsiveReferences,

Should we replace it with:
$responsive !== null ? ($responsive === '' ? '1' : $responsive !== '0') : $wgCiteResponsiveReferences,

I'm not sure if this suggestion would be a good idea. The fundamental problem is that VE currently cannot tell the difference between responsive="" and responsive. From Parsoid's and VisualEditor's perspective both look the same. The suggestion might make one situation slightly better (responsive doesn't become responsive="" any more but responsive="1") but at the same time make other situations worse (responsive="" would also become responsive="1", causing more dirty diffs).

The WMDE-TechWish team did their best to avoid dirty diffs as much as possible in the Cite code we are responsible for. The remaining problem is probably a responsibility of the Content-Platform-Team.

It's not just VE, I don't believe the DOM API (in PHP, JavaScript, etc) provides any way to distinguish these cases. We could chose to serialize empty attributes without the ="" but that also causes dirty diffs, just in the other direction.

We could also add a data-parsoid attribute to track this, to make a dirty diff less likely on serialization. But when VE generates a new node, it doesn't really have a way to indicate whether it wants 'empty attribute syntax' or not.

Change #1227372 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/services/parsoid@master] [Proof of concept] Track empty attributes in data-parsoid

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

Ya, as Scott notes, I think there are two issues here: (a) How can Parsoid preserve the attribute-less form in rt-ing? (b) How can VE tell Parsoid to use the attribute-less form in new content?

If users really want this, we will need to treat extension tag syntax a bit liberally and not impose XML tag requirements on them.

And, if so, I think one clean way to do this is to add additional information in the data-mw object. So, right now, Parsoid has data-mw='{"name":"references","attrs":{"responsive":""}}. Parsoid can introduce a new property novalue-attrs like this: data-mw='{"name":"myext","novalue-attrs":["a","b"],"attrs":{"x":"1", "y":""}} for the invocation of <myext a b x="1" y="">..</my-ext>. Selective serialization can preserve the original ordering of attributes, but the canonical ordering can emit value-less attributes before valued attributes. If we are bikeshedding, alternate names are toggle-attrs, bool-attrs, flag-attrs, .....

So, the only thing left here is a product decision: Do we want to do this?

Do we want to do this?

Maybe I get the question wrong. But I, personally, would classify this more as a bug missing feature and not a product decision. The wikitext feature exists. It's used in tens of thousands of articles (here is an example query just for the <references> tag, and another for frameless <mapframe>s). Almost all software respects it, even including VisualEditor and Parsoid (they don't rewrite such attributes unless the surrounding wikitext is touched). Only a very small slice of the involved software stack needs what is effectively a single bit of additional knowledge.

So, the only thing left here is a product decision: Do we want to do this?

You would certainly make lots of German Wikipedia editors very happy – and probably editors from other wikis as well, as more and more wikis are moving away from {{refllist}} (which was one option to avoid unintended diffs with VE changing <references responsive /> to <references responsive ="" />)

Meanwhile I guess, that the simplemost way is preventing the change by VE from <references responsive /> to <references responsive ="" />
Somewhere it should be coded in the Visual Editor, that this change has to be done when saving the edit using the Visual Editor. Let's find this line or snippet and comment it out.

Or:
If the Visual Editor is doing more changes than only this one, let's add as very last change ever: <references responsive ="" /> to <references responsive />
Okay, this behavior will be a little dirty but should work. With this, the problem is not really a problem any more. But with this we have more time to figure out the "changing bug" and to work on it.

Do we want to do this?

Maybe I get the question wrong. But I, personally, would classify this more as a bug missing feature and not a product decision. .... Only a very small slice of the involved software stack needs what is effectively a single bit of additional knowledge.

That is a fair characterization.

What I meant was: do we want to introduce a first-class representation in the output spec for these? Right now, while Parsoid recognizes it and supports its semantics, if you looked at the HTML, Parsoid HTML clients cannot distinguish between <myext a> and <myext a=""> syntactic representations. Is there value to it?

Looks like there is some value to it for editors as Johannes89 and doctaxon are saying in that it prevents some classes of dirty diffs because VE and Parsoid can now distinguish between those forms during edits. But, if we only cared about the Parsoid-VE combo being able to distinguish between them (and not all the other Parsoid HTML clients being able to distinguish between the two forms), we can also implement this as an internal hack without making it a first-class representation in Parsoid's HTML spec.

I characterized all of this as a product decision. Which users care about the distinction?Do we care about making it a first-class output spec change? if only VE/Parsoid users care, how much is preventing this dirty-diff in *some* VE edits worth the extra engineering (doctaxon: it is not just a single line in VE, it needs changes in Parsoid & VE, and testing, etc.)? We don't have to belabor this at this point. I think we can make a quick call on our end and figure out a path forward.

I agree to @thiemowmde that this should not be framed as a product decision - the feature already exists and is used in tens of thousands of articles across multiple wikis. Almost all of the software stack already handles it correctly. What is missing is effectively a single bit of additional information: Parsoid needs to track whether an attribute was originally written without a value. @cscott's proof-of-concept patch tracking empty attributes in data-parsoid (without requiring a first-class change to Parsoid's output spec) looks like exactly the right minimal approach to me. We do not need a first-class spec change to solve the immediate problem. Let's move forward with the data-parsoid tracking approach and stop the dirty diffs.

Understood and that is what I said. Editing and wt2html is already supported. But, if you later want VE to also save new edits in the valueless form, it needs a way to communicate that with Parsoid and we are just trying to cover both edit scenarios at once. And, sure, I don't have to call it a "product decision" if that is getting in the way. As I said, we'll discuss this and figure out the right solution.

Would it make sense to potentially default Parsoid to render wikitext such that it omits redundant default attribute values? Parsoid inherently needs formatting default choices because when using VisualEditor to create new content, no wikitext exists yet.

We have other formatting defaults already, such as a line break before a heading, and spaces around == Heading ==. It would be easier technically to not insert these spaces, but doing so would be unlike how humans write wikitext. Likewise, while technically easier to serialize as XML attribute="value", the simpler output (from a human perspective) is to omit =" which seems like a reasonable coding style to have as default.

  • We know of cases were insertion of ="" is not what humans expect to see and would not have written themselves.
  • There are most certainly editors who write ="" in wikitext for one reason or another, the suggested change preserves this. (I do doubt whether this is done with intent. Values like title="" or style="" are valid, but unlikely in wikitext input, given they wouldn't have any effect. Templates do often produce title="" and such, but template code isn't relevant here because that's processed when parsing to HTML, not when serialising to wikitext itself. Templates do that to keep logic simpler by not making attributes conditional.)
  • Do we know of cases where VE-supported extension tags have attributes that can be empty and where omitting ="" would be unexpected/undesired by editors and/or where doing so would change behavior in terms of how the Parser extensions interpret the args?

Not that browser devtools in Safari and Chrome both default to visualising DOM nodes without ="" when setting an attribute value to the empty string. This suggests that web developers prefer this.

Screenshot 2026-07-16 at 23.33.52.png (554×714 px, 77 KB)
Screenshot 2026-07-16 at 23.34.06.png (579×985 px, 86 KB)

I think it's reasonable to serialize empty attributes as foo instead of foo="" by default. That seems to match "editor preferences" best, and matches what browser debuggers show, as @Krinkle points out. Selective serialization can handle *most* other cases where an editor explicitly wrote foo="" and we want to preserve that; and adding a data-parsoid attribute preserves the other few cases (ie where other nearby edits were made which preclude using selective serialization).

I think the only reason to add a data-mw attribute is if we want to make it visible to the editor and downstream tools; ie if Visual Editor were to expose a checkbox for each empty extension attribute for "serialize as empty attribute yes/no". I don't think anyone is asking for that level of control. Alternatively, if enterprise or other downstream clients felt that it was semantically important to know which variant the wikitext author used; again, I don't see anyone asking for that either.

https://de.wikipedia.org/w/index.php?title=Wikipedia:Fragen_zur_Wikipedia#Probleme_mit_EN-Links_in_Stadler_Flirt

With this we have a production bug in Firefox now concerning the handling of references and their link click behavior having <references responsive=""> (often set automatically by Visual Editor) or <references responsive="1"> in article text. In my opinion this makes the task higher prior because now it's a bug in production.

But since it works in other browsers (even for Firefox on non-Windows devices), this suggests to me that something is wrong with the Firefox browser itself. Firefox has had several Wikipedia bugs recently.

Change #1325558 had a related patch set uploaded (by Subramanya Sastry; author: Subramanya Sastry):

[mediawiki/services/parsoid@master] WIP: html2wt: Default to emitting compact valueless attributes

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