Page MenuHomePhabricator

PHP Notice: Trying to get property 'parentNode' of non-object
Closed, ResolvedPublicBUG REPORT

Description

Error
normalized_message
[{reqId}] {exception_url}   PHP Notice: Trying to get property 'parentNode' of non-object
exception.trace
from /srv/parsoid-testing/src/Wt2Html/PP/Processors/DOMRangeBuilder.php(1217)
#0 /srv/parsoid-testing/src/Wt2Html/PP/Processors/DOMRangeBuilder.php(1217): MWExceptionHandler::handleError(integer, string, string, integer, array)
#1 /srv/parsoid-testing/src/Wt2Html/PP/Processors/DOMRangeBuilder.php(1237): Wikimedia\Parsoid\Wt2Html\PP\Processors\DOMRangeBuilder->findWrappableTemplateRangesRecursive(Wikimedia\Parsoid\DOM\Compat\Element, array, array)
#2 /srv/parsoid-testing/src/Wt2Html/PP/Processors/DOMRangeBuilder.php(1237): Wikimedia\Parsoid\Wt2Html\PP\Processors\DOMRangeBuilder->findWrappableTemplateRangesRecursive(Wikimedia\Parsoid\DOM\Compat\Element, array, array)
#3 /srv/parsoid-testing/src/Wt2Html/PP/Processors/DOMRangeBuilder.php(1237): Wikimedia\Parsoid\Wt2Html\PP\Processors\DOMRangeBuilder->findWrappableTemplateRangesRecursive(Wikimedia\Parsoid\DOM\Compat\Element, array, array)
#4 /srv/parsoid-testing/src/Wt2Html/PP/Processors/DOMRangeBuilder.php(1110): Wikimedia\Parsoid\Wt2Html\PP\Processors\DOMRangeBuilder->findWrappableTemplateRangesRecursive(Wikimedia\Parsoid\DOM\Compat\Element, array, array)
#5 /srv/parsoid-testing/src/Wt2Html/PP/Processors/AnnotationDOMRangeBuilder.php(31): Wikimedia\Parsoid\Wt2Html\PP\Processors\DOMRangeBuilder->findWrappableMetaRanges(Wikimedia\Parsoid\DOM\Compat\Element)
#6 /srv/parsoid-testing/src/Wt2Html/PP/Processors/AnnotationDOMRangeBuilder.php(185): Wikimedia\Parsoid\Wt2Html\PP\Processors\AnnotationDOMRangeBuilder->wrapAnnotationsInTree(Wikimedia\Parsoid\DOM\Compat\Element)
#7 /srv/parsoid-testing/src/Wt2Html/PP/Processors/WrapAnnotations.php(22): Wikimedia\Parsoid\Wt2Html\PP\Processors\AnnotationDOMRangeBuilder->execute(Wikimedia\Parsoid\DOM\Compat\Element)
#8 /srv/parsoid-testing/src/Wt2Html/DOMPostProcessor.php(160): Wikimedia\Parsoid\Wt2Html\PP\Processors\WrapAnnotations->run(Wikimedia\Parsoid\Config\Env, Wikimedia\Parsoid\DOM\Compat\Element, array, boolean)
#9 /srv/parsoid-testing/src/Wt2Html/DOMPostProcessor.php(979): Wikimedia\Parsoid\Wt2Html\DOMPostProcessor->Wikimedia\Parsoid\Wt2Html\{closure}(Wikimedia\Parsoid\DOM\Compat\Element, array, boolean)
#10 /srv/parsoid-testing/src/Wt2Html/DOMPostProcessor.php(1033): Wikimedia\Parsoid\Wt2Html\DOMPostProcessor->doPostProcess(Wikimedia\Parsoid\DOM\Compat\Element)
#11 /srv/parsoid-testing/src/Wt2Html/DOMPostProcessor.php(1051): Wikimedia\Parsoid\Wt2Html\DOMPostProcessor->process(Wikimedia\Parsoid\DOM\Compat\Element)
#12 /srv/parsoid-testing/src/Wt2Html/ParserPipeline.php(178): Wikimedia\Parsoid\Wt2Html\DOMPostProcessor->processChunkily(string, array)
#13 /srv/parsoid-testing/src/Wt2Html/ParserPipelineFactory.php(308): Wikimedia\Parsoid\Wt2Html\ParserPipeline->parseChunkily(string, array)
#14 /srv/parsoid-testing/src/Core/WikitextContentModelHandler.php(106): Wikimedia\Parsoid\Wt2Html\ParserPipelineFactory->parse(string)
#15 /srv/parsoid-testing/src/Parsoid.php(166): Wikimedia\Parsoid\Core\WikitextContentModelHandler->toDOM(Wikimedia\Parsoid\Config\Env)
#16 /srv/parsoid-testing/src/Parsoid.php(198): Wikimedia\Parsoid\Parsoid->parseWikitext(MWParsoid\Config\PageConfig, array)
#17 /srv/parsoid-testing/extension/src/Rest/Handler/ParsoidHandler.php(589): Wikimedia\Parsoid\Parsoid->wikitext2html(MWParsoid\Config\PageConfig, array, NULL)
#18 /srv/parsoid-testing/extension/src/Rest/Handler/TransformHandler.php(119): MWParsoid\Rest\Handler\ParsoidHandler->wt2html(MWParsoid\Config\PageConfig, array, string)
#19 /srv/mediawiki/php-1.38.0-wmf.7/includes/Rest/Router.php(403): MWParsoid\Rest\Handler\TransformHandler->execute()
#20 /srv/mediawiki/php-1.38.0-wmf.7/includes/Rest/Router.php(330): MediaWiki\Rest\Router->executeHandler(MWParsoid\Rest\Handler\TransformHandler)
#21 /srv/mediawiki/php-1.38.0-wmf.7/includes/Rest/EntryPoint.php(165): MediaWiki\Rest\Router->execute(MediaWiki\Rest\RequestFromGlobals)
#22 /srv/mediawiki/php-1.38.0-wmf.7/includes/Rest/EntryPoint.php(130): MediaWiki\Rest\EntryPoint->execute()
#23 /srv/mediawiki/php-1.38.0-wmf.7/rest.php(31): MediaWiki\Rest\EntryPoint::main()
#24 /srv/mediawiki/w/rest.php(3): require(string)
#25 {main}
Impact
Notes

Details

Related Objects

StatusSubtypeAssignedTask
OpenReleaseNone
OpenNone
OpenNone
OpenNone
OpenFeatureNone
OpenNone
OpenNone
OpenNone
OpenNone
OpenNone
ResolvedEsanders
OpenFeatureNone
Resolvedihurbain
Resolvedihurbain
ResolvedBUG REPORTihurbain

Event Timeline

ssastry triaged this task as Medium priority.Nov 6 2021, 6:39 PM
ssastry created this task.

Related error message: Argument 1 passed to Wikimedia\Parsoid\Utils\DOMDataUtils::getDataParsoid() must be an instance of Wikimedia\Parsoid\DOM\Compat\Element, null given, called in /srv/parsoid-testing/src/Wt2Html/PP/Processors/DOMRangeBuilder.php on line 1217

ssastry changed the subtype of this task from "Production Error" to "Bug Report".Nov 7 2021, 3:52 AM

Change 737441 had a related patch set uploaded (by Isabelle Hurbain-Palatin; author: Isabelle Hurbain-Palatin):

[mediawiki/services/parsoid@master] Fix global state of annotation id and DOMPostProcessor pass order

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

Change 737441 merged by jenkins-bot:

[mediawiki/services/parsoid@master] Fix global state of annotation id and DOMPostProcessor pass order

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

Just these pages:

metawiki:Communications/Communicating_about_the_Wikimedia_Foundation 
metawiki:Wikimedia_Community_User_Group_Hong_Kong 
metawiki:Grants
metawiki:Strategy/Wikimedia_movement/2017/Sources/58_expert_summaries_(June_2017) 
metawiki:FMCD

The first three are following the same pattern:

{|
| <translate><tvar|somevar>stuff</></translate>
|}

The pipe in the tvar definition parses as "this is styling for the table cell" and the translate tag becomes pulled into that, and the end translate tag gets lost. We explicitly do not support <tvar| anymore, so I'd argue these should be fixed.

The fourth one has actual styling with a pipe, but the <translate> tag is roped in there (and probably shouldn't) ("<translate> <!--T:408--> rowspan="1" |The Government of ..."), so I'd argue in favor of fixing the wikitext there as well to be honest.

The last one I haven't had time to look exactly yet - it seems less obvious, though.

In any case, there seems to be a case in favor of "detecting cases where we have a range end and no range start" (even if the <translate> extension kind of guarantees we should always have balanced tags) for the explicit case of "annotation open in an attribute, closed after the attribute in question".

Aha .. so, this is a real issue since in the core parser, the translate tag kicks in and processes its tags before the page is parsed. But, in Prasoid, it all happens at the same time in the tokenizer. So, the precedence for parsing annotations should be fixed so it takes precedence over everything else that might recognize a "|". I haven't stared at the tokenizer in a long time, but, @Arlolra might have a quicker suggestion for you there.

I edited that last page to get rid of useless tables with lots of fostered content.

The precedence thing is not obvious to me...
If we have something like "| <translate>color: red</translate> | stuff", we'd actually want | to be parsed before <translate> so that the translate annotation would be seen as part of the td attribute...

Change 737725 had a related patch set uploaded (by Arlolra; author: Arlolra):

[mediawiki/services/parsoid@master] [WIP] Extension tags with | in their name

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

Anyway, Isabelle reminded me that we have T274881: Change translation variable (tvar) syntax which changed the syntax of tvars and the crashers are because we don't recognize them and that changes how the "|" is now parsed that causes other impacts in a table. So, ideally, yes, we would fix those pages to use the new tvar syntax.

Change 737725 merged by jenkins-bot:

[mediawiki/services/parsoid@master] Hack: Add partial support for older tvar syntax to prevent crashers

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

This is mostly resolved now. There are still a couple of crashers

The problem of the first one is not that we split the translate range across table cells, it's that the translate range contains both attributes of the <td> and the content of the cell. I do agree that, in this specific case, pulling the <translate> tag in the table cell with the content would be the right move, but I don't think this is a call we can make for every such instance, or even such rowspan instance (because I could see scenarios where the table structure may depend on the language...).

I think the semantically correct way of handling that (to not lose potential translation of the attributes) would be to parse this as if we had | <translate> <!--T:374--> rowspan="1"</translate> | <translate>Information is universal and Wikipedia has the right idea to allow everyone to be able to contribute. It has made research and is a good access point to information. </translate>. I'm not yet at the "how to do that", though 😄

For the second one, the structure is

{{<tvar|WikilinkBlogsRedactors>Wwc-link|wikilink=WikiWomen's Collaborative/Blogs|link text=</>
share on the blog
}}

which has the triple combo "tvar with old syntax / template with annotation spanning several arguments / argument becoming a link attribute"..... this is going to be a tough one.

In general, we want all annotations markup to be structured and respect structure and the rowspan example also breaks that. I don't think we want table structure to change based on language. That then stops being an annotation.

More specifically for <translate>, those annotations are pertinent only to text nodes for translation ... so, it should ideally only wrap text nodes, and not include markup ... introducing markup breaks tokenizing in bad ways. So, at least for the translate annotation, I would say the solution is to prevent crashers and throw this back to editors to fix the page.

Change 742506 had a related patch set uploaded (by Isabelle Hurbain-Palatin; author: Isabelle Hurbain-Palatin):

[mediawiki/services/parsoid@master] Avoid crashers on bad annotation nesting

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

Change 742506 merged by jenkins-bot:

[mediawiki/services/parsoid@master] Avoid crashers on bad annotation nesting

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

Change 742570 had a related patch set uploaded (by Sbailey; author: Sbailey):

[mediawiki/vendor@master] Bump Parsoid to 0.15.0-a11

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

Change 742570 merged by jenkins-bot:

[mediawiki/vendor@master] Bump Parsoid to 0.15.0-a11

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