See also T14752: Space before/after »guillemets« (»/«) converted to non-breaking space ( ) (French spaces), T60529: Non-breaking thin spaces before double punctuation marks (French spaces).

I suspect I fixed this in T197902: Be more selective in applying French Space armoring; »quote« shouldn't add   anymore.

You're right, I didn't notice it in the example, and i missed it in @matmarex's screenshot, because data-mw was abbreviated in both cases. But it's there, on the <h2> tag in your example, and on the top <h2> in F31045488.

@subbu -- the data-mw attribute necessary to round-trip the template is only occuring on the <section> tag. That's true for both the template example in https://www.mediawiki.org/wiki/Parsing/Notes/Section_Wrapping#Examples and the extension example in @matmarex's F31045488.

Seems like the bug is in VE's section-stripping code? It should probably not strip the section tag iff it has typeof="mw:Transclusion"; that will probably be handled properly by the existing VE "template-affected tag" mechanisms (which just look at typeof and ignore the tag name AFAIK).

If it were possible to direct writes at some *other* database before pointing them at the live database, that might be a good "step #0".

It gets worse:

>>> $doc = DOMDocument::loadHTML('<p>foo');  null;
=> null
>>> $node = $doc->createElement('math'); null;
=> null
>>> $node->setAttribute('xmlns', 'foo');
=> true
>>> $node->getAttribute('xmlns')
=> "foo"
>>> $node->getAttributeNode('xmlns')
=> DOMNameSpaceNode {#2310
     +nodeName: "xmlns",
     +nodeValue: "",
     +nodeType: XML_NAMESPACE_DECL_NODE,
     +prefix: "",
     +localName: "xmlns",
     +namespaceURI: "foo",
     +ownerDocument: DOMDocument {#2305 …},
     +parentNode: null,
   }
>>> $node->setAttribute('x','y');
=> DOMAttr {#2316
...
   }
>>> $node->attributes
=> DOMNamedNodeMap {#2318
     +length: 1,
   }
>>> $node->attributes->item(0)
=> DOMAttr {#2316
...
   }
>>> $node->getAttributeNode('x')
=> DOMAttr {#2316
     +nodeName: "x",
     +nodeValue: "",
     +nodeType: XML_ATTRIBUTE_NODE,
     +parentNode: DOMElement {#2321 …},
[...]
     +namespaceURI: null,
     +prefix: "",
     +localName: "x",
     +baseURI: null,
     +textContent: "",
     +name: "x",
     +specified: true,
     +value: "y",
     +ownerElement: DOMElement {#2321 …},
     +schemaTypeInfo: null,
   }
>>> $node->getAttributeNode('xmlns') instanceof \DOMAttr
=> false
>>>

Note that the attribute node for the xmlns attribute is a DOMNameSpaceName, and the value of the attribute is stored in namespaceURI, not value. All the other attributes are instances of DOMAttr with value in value.

(The reason PHP's magic behavior is awful is that the attribute "exists" in some sense, but it's completely invisible to DOMNode::setAttribute/DOMNode::hasAttribute/etc. So even if we hacked our serializer to behave the same way that $doc->saveXML does and magically 'revive' the attribute, I'd have to go through and audit every setAttribute/hasAttribute etc in Parsoid to make sure that a hidden xmlns attribute isn't going to break things by being invisible yet present...)

Yeah, but that's not how the DOM is supposed to work.

https://github.com/php/php-src/blob/php-7.2.24/ext/dom/element.c#L411

Another one: T235295: MathML tags are missing xmlns attribute -- PHP's DOM refuses to accept an attribute named xmlns for some reason:

$ psysh 
Psy Shell v0.9.9 (PHP 7.3.4-2 — cli) by Justin Hileman
>>> $doc = new \DOMDocument(); null
=> null
>>> $node = $doc->createElement('math'); null;
=> null
>>> $node->setAttributeNS('http://www.w3.org/2000/xmlns/', 'xmlns', 'http://www.w3.org/1998/Math/MathML');
=> null
>>> $node->attributes
=> DOMNamedNodeMap {#2335
     +length: 0,
   }
>>> $node->setAttribute('xmlns', 'xyz');
=> false
>>> $node->attributes
=> DOMNamedNodeMap {#2333
     +length: 0,
   }
>>> $node->setAttribute('x', 'xyz'); null;
=> null
>>> $node->attributes
=> DOMNamedNodeMap {#2322
     +length: 1,
   }
>>> $node2 = $doc->createElementNS('http://www.w3.org/1999/xhtml', 'math'); null;

Hm. Apparently the PHP DOM just ignores setAttribute and setAttributeNS when the name is xmlns. This is regardless of whether the element is created with createElement or createElementNS. I haven't figured out a workaround yet.

I bet the culprit is the workaround for T217708 (Remex commit 33de7ba9746fce0aaaeb9314a7a78460f2a28122), although that was *supposed* to affect only HTML elements, not xmlns="http://www.w3.org/1998/Math/MathML".

No one has audited core or extensions for this particular usage pattern. So we assume there is at least *some* code in core/extensions which would be affected.

In T236810#5616875, @TK-999 wrote:

@cscott I was wondering if there is a preferred migration path away from Parser::replaceLinkHolders for code that relied on it. I see that the mLinkHolders field in Parser is public, but I imagine that won't stay that way either, right? Thanks in advance!

@Od1n could you do some quick benchmarks to satisfy the reviewer on the patch above?

Hm. Where should the ref go -- does PHP actually generate an entry in <referernces/> for this (gosh, I hope so...)?

I think this was a function which is in core and was ported to JS?

We have <section id="0">....</section><section id="981">.... and they appear to count up from there, but they must jump because we end with <section id="1330">....</section><section id="-1">....</section>.

Hm, that's strange: the section ids in the legacy parser only go up to 31:

So the main body of the gallery has sensible DSRs:

<body data-parsoid='{"dsr":[0,84,0,0]}'
<ul class="gallery mw-gallery-traditional" typeof="mw:Extension/gallery" data-parsoid='{"dsr":[0,83,9,10]}'
<div class="gallerytext" data-parsoid='{"dsr":[23,72,0,0]}'
<sup class="mw-ref" id="cite_ref-1" typeof="mw:Extension/ref" data-parsoid='{"dsr":[34,72,5,6]}'

And other outer wrapper of the auto-inserted <references/> is zero-width, but it contains the link with the bogus DSR (which fwiw is outside the boundary of its parent):

<div class="mw-references-wrap" typeof="mw:Extension/references" data-parsoid='{"dsr":[84,84,0,0]}'
<a rel="mw:ExtLink" href="https://cscott.net" class="external text" data-parsoid='{"dsr":[46,73,20,1]}'>

in PHP that last is:

<a rel="mw:ExtLink" href="https://cscott.net" class="external text" data-parsoid='{"dsr":[46,73,18,1]}'

...but both are bogus. The string at [46,73] is //cscott.net foo💩 ]</ref>\n and neither the first 18 or 20 characters of that are a sensible "open tag" width.

This turns out to be an instance of the dreaded "PHP regex D modifier" bug.

Ok, I can reproduce it (parsertest in gerrit above), and confirmed that this is a Parsoid/PHP bug. Now to figure out how to fix it....

for lnwiki, looks like the link*trail* is getting stuck on as a prefix? Is that what that looks like to you?

Probably missing implementations of the JSON and ProofRead page extensions...

I can reproduce (at least part of) the DSR differences with the following wikitext:

<gallery>
File:Foo.jpg|💩 caption <ref>[https://cscott.net foo💩 ]</ref>
</gallery>

– from kawiki is \u2013; — from arwiki is \u2014. Both of these are in the link prefix range of \x80-\x10fffff, but even if we're not propagating the u modifier across, it would be parsed as [\x80-\xDBFF\xDFFF] and \u2013/\u2014 ought to be in that range.

Link prefix/trail on arwiki is:

"linkprefixcharset": "a-zA-Z\\x{80}-\\x{10ffff}",
"linkprefix": "/^((?>.*[^a-zA-Z\\x{80}-\\x{10ffff}]|))(.+)$/sDu",
"linktrail": "/^([a-z\u0621-\u064a]+)(.*)$/sDu",

Link prefix/trail on kawiki is:

"linkprefixcharset": "a-zA-Z\\x{80}-\\x{10ffff}",
"linkprefix": "/^((?>.*[^a-zA-Z\\x{80}-\\x{10ffff}]|))(.+)$/sDu",
"linktrail": "/^([a-z\u10d0\u10d1\u10d2\u10d3\u10d4\u10d5\u10d6\u10d7\u10d8\u10d9\u10da\u10db\u10dc\u10dd\u10de\u10df\u10e0\u10e1\u10e2\u10e3\u10e4\u10e5\u10e6\u10e7\u10e8\u10e9\u10ea\u10eb\u10ec\u10ed\u10ee\u10ef\u10f0\u201c\u00bb]+)(.*)$/sDu"

Well, 1.34.0 is due in November, so it seems we don't actually have to wait that long to fix T228881. In fact, since 1.34 has already forked and @Reedy has done the backports, it could be landed on master now. I'd prefer waiting a few weeks just to let the deprecation code ride the train and get testing and make sure @Fomafix and I haven't inadvertently regressed something with our revert + deprecation patches, but early November seems reasonable to land https://gerrit.wikimedia.org/r/544249.

In T235392#5588105, @Reedy wrote:

This is all backported

Here's my draft list of scenarios to test, from our hangout the other day:

So to be concrete, I'm suggesting to merge @Fomafix's revert https://gerrit.wikimedia.org/r/543001 and then my deprecation patch on top of that https://gerrit.wikimedia.org/r/543903 but *not* the hasTitle and return type hint patches https://gerrit.wikimedia.org/r/543002 and https://gerrit.wikimedia.org/r/543003.

In T235392#5573013, @Foxtrott wrote:

What about checking for $mTitle === null and issuing a deprecation warning?

It's easy enough in this instance, and if it helps reduce noise in the HTML diffs it helps increase our confidence in deploying Parsoid/PHP so I think it's still worthwhile.

Ok, tracked it down to:

Sanitizer.normalizeSectionIdWhiteSpace = function(id) {
	return id.replace(/[ _]+/g, ' ').trim();
};

vs

public static function normalizeSectionIdWhiteSpace( string $id ): string {
		return trim( preg_replace( '/[ _]+/', ' ', $id ) );
}

Minimum repro:

$ echo '==={{CHNML}}}===' | php bin/parse.php --domain zh.wikipedia.org --body_only
<h3 id="_中国大陆}" data-parsoid='{"dsr":[0,16,3,3]}'><span id="_.E4.B8.AD.E5.9B.BD.E5.A4.A7.E9.99.86.7D" typeof="mw:FallbackId" data-parsoid='{"dsr":[3,3,null,null]}'></span><span class="flagicon" about="#mwt1" typeof="mw:Transclusion" data-parsoid='{"stx":"html","dsr":[3,12,null,null],"pi":[[]]}' data-mw='{"parts":[{"template":{"target":{"wt":"CHNML","href":"./Template:CHNML"},"params":{},"i":0}}]}'><figure-inline class="mw-image-border" typeof="mw:Image"><span><img alt="" resource="./File:Flag_of_the_People's_Republic_of_China.svg" src="//upload.wikimedia.org/wikipedia/commons/thumb/f/fa/Flag_of_the_People%27s_Republic_of_China.svg/22px-Flag_of_the_People%27s_Republic_of_China.svg.png" data-file-width="900" data-file-height="600" data-file-type="drawing" height="15" width="22" srcset="//upload.wikimedia.org/wikipedia/commons/thumb/f/fa/Flag_of_the_People%27s_Republic_of_China.svg/44px-Flag_of_the_People%27s_Republic_of_China.svg.png 2x, //upload.wikimedia.org/wikipedia/commons/thumb/f/fa/Flag_of_the_People%27s_Republic_of_China.svg/33px-Flag_of_the_People%27s_Republic_of_China.svg.png 1.5x"/></span></figure-inline><span typeof="mw:Entity"> </span></span><a rel="mw:WikiLink" href="./中国大陆" title="中国大陆" about="#mwt1" data-parsoid='{"stx":"piped","a":{"href":"./中国大陆"},"sa":{"href":"中国大陆"}}'>中国大陆</a>}</h3>

-vs-

$ echo '==={{CHNML}}}===' | bin/parse.js --domain zh.wikipedia.org --body_only
<h3 id="中国大陆}" data-parsoid='{"dsr":[0,16,3,3]}'><span id=".E4.B8.AD.E5.9B.BD.E5.A4.A7.E9.99.86.7D" typeof="mw:FallbackId" data-parsoid='{"dsr":[3,3]}'></span><span class="flagicon" about="#mwt1" typeof="mw:Transclusion" data-parsoid='{"stx":"html","dsr":[3,12,null,null],"pi":[[]]}' data-mw='{"parts":[{"template":{"target":{"wt":"CHNML","href":"./Template:CHNML"},"params":{},"i":0}}]}'><figure-inline class="mw-image-border" typeof="mw:Image"><span><img alt="" resource="./File:Flag_of_the_People's_Republic_of_China.svg" src="//upload.wikimedia.org/wikipedia/commons/thumb/f/fa/Flag_of_the_People%27s_Republic_of_China.svg/22px-Flag_of_the_People%27s_Republic_of_China.svg.png" data-file-width="900" data-file-height="600" data-file-type="drawing" height="15" width="22"/></span></figure-inline><span typeof="mw:Entity"> </span></span><a rel="mw:WikiLink" href="./中国大陆" title="中国大陆" about="#mwt1" data-parsoid='{"stx":"piped","a":{"href":"./中国大陆"},"sa":{"href":"中国大陆"}}'>中国大陆</a>}</h3>

Ah. Is the value always '' in cases where the diff appears? It seemed like you were saying that in some cases the unicode space wasn't being properly (?) stripped from the value. (That seems to be the case in T235684, for example.)

Which of these (from your second example) is actually correct?

{"k":"div 1","named":true,"spc":["","","","   \n"]},
{"k":"div 1","named":true,"spc":["","","  ","\n"]},

Either the whitespace appears both before *and* after div 1 in the wikitext (PHP) or it only appears after div 1 (JS). One of those has to be wrong, and we should fix it.

the regular id attribute seems to be different as well -- looks like we're not doing whitespace stripping on the left hand side appropriately.

hm, it *should* be converted. I wrote the recursive code. Maybe there's a case missing somewhere. Will look into it...

So...

<span vocab="http://schema.org" typeof="Comment" class="mw-signature">
    <span rel="creator" resource="/wiki/User:cscott"> <!-- machine readable link to username in span attributes -->
       <!-- But note that everything in this <span> is customizable text, don't try to parse inside the tag -->
       <a href="/wiki/User:cscott" title="User:cscott">C. Scott Ananian</a> (<a href="/wiki/User_talk:cscott" title="User talk:cscott">talk</a>)
     </span>
    <time property="dateCreated" datetime="2007-03-29T18:07Z">18:07, 29 March 2007 (UTC)</time>
</span>

?
(You could also add the userid as an attribute to the <span> if that was thought useful.)

In T234979#5564936, @ssastry wrote:

These diffs should be normalized away

ssastry@scandium:/srv/deployment/parsoid/deploy/src/bin$ node diff.html.js diff.yaml fr.wikipedia.org Paris
DIFFS FOR fr.wikipedia.org:Paris
...
----- JS:[94991, 95143] -----
<li class="gallerybox" style="width: 477.3333333333333px;" data-parsoid="{}">
<div class="thumb" style="width: 475.3333333333333px;" data-parsoid="{}">
+++++ PHP:[94895, 95043] +++++
<li class="gallerybox" style="width: 477.33333333333px;" data-parsoid="{}">
<div class="thumb" style="width: 475.33333333333px;" data-parsoid="{}">

subbu: would percent-encoding the . as %2E help, or is this IE6/7 hack done after percent-decoding?

To be clear (and I'll edit this if I turn out to be wrong), this is a *future* not *current* problem. It will break when we switch to Parsoid/PHP from Parsoid/JS.

Quoting the commit down here in comments so it autolinks properly and I can click through to see what it is: 4b9344af71c69a49735eddae3278e4d6460532e4

To restate, I'm proposing that we take the DisplaySpace hack *out* of the tokenizer, and instead run it as a DOMPostProcessor pass, with a corresponding preprocessor in the html2wt side to reverse that transformation.

From Parsoid's perspective:

What about splitting the hosts, and directing JS traffic to one half of them and PHP traffic to the other half? Then by changing exactly what machines are in each pool we can reallocate resources as necessary, since every machine would be capable of responding to either PHP or JS requests.

See also T233818: Call to a member function getContent() on null.

I think the "proper" solution would be to carry around some sort of parameter -- maybe even all the way from the REST API in terms of a special path or query parameter -- to allow us to distinguish the case where a revision not being found is "expected" (ie, new page creation), from where revision not being found should be "tolerated" (ie, it's always possible there's a race between saving a new revision and someone deleting the page -- but ideally in this case we'll have the old revision ourself and pass it in to the API, so that we can generate proper wikitext w/o having to fetch the original revision), from where revision not being found/supplied is definitely an bug that should generate a 500 error.

Well, we have to be able to create a page from scratch (with no previous revision). So a zero-length string seems reasonable to me as a fallback, in both the Api config and the integrated config.