https://logstash.wikimedia.org/app/kibana#/doc/logstash-*/logstash-2019.12.05/parsoid-php?id=AW7W3Pnjh3Uj6x1zFgqw&_g=h@44136fa

Return value of Parsoid\Html2Wt\SerializerState::getOrigSrc() must be of the type string or null, boolean returned

#0 /srv/deployment/parsoid/deploy-cache/revs/0910e18d8477587cf44c17a2d500e0f73836c0a6/src/src/Html2Wt/DOMHandlers/DOMHandler.php(345): Parsoid\Html2Wt\SerializerState->getOrigSrc(integer, integer)
#1 /srv/deployment/parsoid/deploy-cache/revs/0910e18d8477587cf44c17a2d500e0f73836c0a6/src/src/Html2Wt/DOMHandlers/HeadingHandler.php(31): Parsoid\Html2Wt\DOMHandlers\DOMHandler->getLeadingSpace(Parsoid\Html2Wt\SerializerState, DOMElement, string)
#2 /srv/deployment/parsoid/deploy-cache/revs/0910e18d8477587cf44c17a2d500e0f73836c0a6/src/src/Html2Wt/WikitextSerializer.php(1274): Parsoid\Html2Wt\DOMHandlers\HeadingHandler->handle(DOMElement, Parsoid\Html2Wt\SerializerState, boolean)
#3 [internal function]: Parsoid\Html2Wt\WikitextSerializer->serializeDOMNode(DOMElement, Parsoid\Html2Wt\DOMHandlers\HeadingHandler)
#4 /srv/deployment/parsoid/deploy-cache/revs/0910e18d8477587cf44c17a2d500e0f73836c0a6/src/src/Html2Wt/WikitextSerializer.php(1372): call_user_func(array, DOMElement, Parsoid\Html2Wt\DOMHandlers\HeadingHandler)
#5 /srv/deployment/parsoid/deploy-cache/revs/0910e18d8477587cf44c17a2d500e0f73836c0a6/src/src/Html2Wt/SerializerState.php(669): Parsoid\Html2Wt\WikitextSerializer->serializeNode(DOMElement)
#6 /srv/deployment/parsoid/deploy-cache/revs/0910e18d8477587cf44c17a2d500e0f73836c0a6/src/src/Html2Wt/SerializerState.php(692): Parsoid\Html2Wt\SerializerState->serializeChildren(DOMElement, NULL)
#7 /srv/deployment/parsoid/deploy-cache/revs/0910e18d8477587cf44c17a2d500e0f73836c0a6/src/src/Html2Wt/WikitextSerializer.php(1645): Parsoid\Html2Wt\SerializerState->kickOffSerialize(DOMElement)
#8 /srv/deployment/parsoid/deploy-cache/revs/0910e18d8477587cf44c17a2d500e0f73836c0a6/src/src/Html2Wt/SelectiveSerializer.php(109): Parsoid\Html2Wt\WikitextSerializer->serializeDOM(DOMElement, boolean)
#9 /srv/deployment/parsoid/deploy-cache/revs/0910e18d8477587cf44c17a2d500e0f73836c0a6/src/src/WikitextContentModelHandler.php(109): Parsoid\Html2Wt\SelectiveSerializer->serializeDOM(DOMElement)
#10 /srv/deployment/parsoid/deploy-cache/revs/0910e18d8477587cf44c17a2d500e0f73836c0a6/src/src/Parsoid.php(206): Parsoid\WikitextContentModelHandler->fromHTML(Parsoid\Config\Env, DOMDocument, Parsoid\SelserData)
#11 /srv/deployment/parsoid/deploy-cache/revs/0910e18d8477587cf44c17a2d500e0f73836c0a6/src/extension/src/Rest/Handler/ParsoidHandler.php(826): Parsoid\Parsoid->html2wikitext(MWParsoid\Config\PageConfig, string, array, Parsoid\SelserData)
#12 /srv/deployment/parsoid/deploy-cache/revs/0910e18d8477587cf44c17a2d500e0f73836c0a6/src/extension/src/Rest/Handler/TransformHandler.php(95): MWParsoid\Rest\Handler\ParsoidHandler->html2wt(Parsoid\Config\Env, array, string)
#13 /srv/mediawiki/php-1.35.0-wmf.8/includes/Rest/Router.php(315): MWParsoid\Rest\Handler\TransformHandler->execute()
#14 /srv/mediawiki/php-1.35.0-wmf.8/includes/Rest/Router.php(285): MediaWiki\Rest\Router->executeHandler(MWParsoid\Rest\Handler\TransformHandler)
#15 /srv/mediawiki/php-1.35.0-wmf.8/includes/Rest/EntryPoint.php(113): MediaWiki\Rest\Router->execute(MediaWiki\Rest\RequestFromGlobals)
#16 /srv/mediawiki/php-1.35.0-wmf.8/includes/Rest/EntryPoint.php(80): MediaWiki\Rest\EntryPoint->execute()
#17 /srv/mediawiki/php-1.35.0-wmf.8/rest.php(31): MediaWiki\Rest\EntryPoint::main()
#18 /srv/mediawiki/w/rest.php(3): require(string)
#19 {main}

I can only reproduce it on a single page and only on officewiki,

I can reproduce it locally by starting with creating a blank page and then editing with the above steps.

return $start <= $end ? substr( $this->selserData->oldText, $start, $end - $start ) : null;

and then:

If string is less than start characters long, FALSE will be returned.
If length is given and is negative, then that many characters will be omitted from the end of string (after the start position has been calculated when a start is negative). If start denotes the position of this truncation or beyond, FALSE will be returned.

So, it must be that the string is less than $start characters long, so we're returning FALSE and crashing.

Note that if we "fix" this to return null, the DOMHandlers::getLeadingSpace() will set $space to null and then return that, crashing again at the exit where the return value is type checked to string.

So, it must be that the string is less than $start characters long, so we're returning FALSE and crashing.

strlen( $this->selserData->oldText ) === 0 for a blank page so, yes, trivially.

Note that if we "fix" this to return null, the DOMHandlers::getLeadingSpace() will set $space to null and then return that, crashing again at the exit where the return value is type checked to string.

Yes, but return $start <= $end ? substr( $this->selserData->oldText, $start, $end - $start ) : null; already returns null if the first condition isn't met, so it's doubly broken.

But, more to the point, I imagine what's happening is that when you paste the content from the doc, == test banana ==, it helpfully parses the wikitext syntax and leaves data-parsoid attributes behind, which are then used during serialization. So, this is more insidious than it may appear, because the dsr values would correspond to the wikitext from the snippit, not the page.

So VE bug, in that it should strip data-parsoid during paste?

(Still probably my fault, since I think I wrote that particular bit of VE code, but still.)

Checking copy(ve.init.target.docToSave.body.outerHTML), we get,

<body data-parsoid="{&quot;dsr&quot;:[0,0,0,0]}" lang="en" class="mw-content-ltr sitedir-ltr ltr mw-body-content parsoid-body mediawiki mw-parser-output" dir="ltr"><p></p><h2 id="test_banana" data-parsoid="{&quot;dsr&quot;:[0,17,2,2]}">test banana</h2></body>

(Still probably my fault, since I think I wrote that particular bit of VE code, but still.)

Before we point fingers, I should see if the same thing happens when RESTBase is involved, since data-parsoid won't be inlined there. Could be reason why @Quiddity couldn't reproduce on a public wiki. Maybe VE just expects to have to remove the ids that are returned.

Change 554965 had a related patch set uploaded (by Arlolra; owner: Arlolra):
[mediawiki/services/parsoid@master] Account for getOrigSrc returning null

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

Maybe VE just expects to have to remove the ids that are returned.

That seems to be the case. If I enable RESTBase locally, the Parsoid ids are dropped as expected.

However, note that this isn't specific to pasted content. VE leaves around inlined data-parsoid on any edited content. It must just assume that all data-parsoid is external now.

In T239875#5716983, @Arlolra wrote:

Maybe VE just expects to have to remove the ids that are returned.

That seems to be the case. If I enable RESTBase locally, the Parsoid ids are dropped as expected.

However, note that this isn't specific to pasted content. VE leaves around inlined data-parsoid on any edited content. It must just assume that all data-parsoid is external now.

I am assuming this is because of the need to preserved syntactic variations (of pasted content) that are currently stored in data-parsoid properties. So, this is a trickier problem. Maybe VE needs to signal the cloned nature of data-parsoid with an attribute or in some other fashion.

I am assuming this is because of the need to preserved syntactic variations (of pasted content) that are currently stored in data-parsoid properties. So, this is a trickier problem. Maybe VE needs to signal the cloned nature of data-parsoid with an attribute or in some other fashion.

No, VE faithful strips parsoid ids in the RESTBase scenario for both pasted and edited content. And it leaves behind inline data-parsoid attributes in both cases.

Change 554965 merged by jenkins-bot:
[mediawiki/services/parsoid@master] Account for getOrigSrc returning null

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

Mentioned in SAL (#wikimedia-operations) [2019-12-11T21:40:32Z] <arlolra> Updated Parsoid to af576d5 (T237693, T238777, T237306, T239875, T240053)

Seems like either:

1. VE should strip inline data-parsoid to be consistent in the RESTBase and non-RESTBase scenarios, or
2. We need to rethink how we handle the non-RESTBase case and see if we can make it more similar to the RESTBase case (ie, fetch separate data-parsoid in both cases, and do the recombination client-side before submission the same way RESTBase would)

We will look at this next fiscal year