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

Assigned To

Authored By

	ssastry
	Nov 6 2021, 6:39 PM

Description

Error

mwversion: 1.38.0-wmf.7
reqId: YYbKQg1yesgjP3aWJ9@vjQAAABI
Find reqId in Logstash
Find normalized_message in Logstash

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

Request URL: https://meta.wikimedia.orghttp//meta.wikimedia.org/w/rest.php/meta.wikimedia.org/v3/transform/wikitext/to/pagebundle/Wikimedia_wikis%23Private_wikis

Related Changes in Gerrit:

Subject	Repo	Branch	Lines +/-
Bump Parsoid to 0.15.0-a11	mediawiki/vendor	master	+7 K -5 K
Avoid crashers on bad annotation nesting	mediawiki/services/parsoid	master	+28 -1
Hack: Add partial support for older tvar syntax to prevent crashers	mediawiki/services/parsoid	master	+549 -399
Fix global state of annotation id and DOMPostProcessor pass order	mediawiki/services/parsoid	master	+50 -15

Customize query in gerrit

Related Objects
Search...

Status	Subtype	Assigned	Task
Open	Release	None	T84936 Release VisualEditor-MediaWiki as "1.0"
Open		None	T50429 [Epic] Support editing parts of a page in VisualEditor-MediaWiki
Open		None	T54365 Explore performance gains from progressive (JIT?) de-alienation in VisualEditor
Open		None	T396539 Introduce a way to edit a span of text from within read mode
Open		None	T385558 Improve copy and paste from read mode to VE
Open		None	T174303 Copy-pasting linked ISBN numbers from view mode HTML into VisualEditor inserts wikitext links to Special:BookSources (it should turn them into magic links?)
Open	Feature	None	T54091 The read HTML should have hinting to allow full DOM copying (as opposed to just rich copying) from read mode into VE surfaces
Open		None	T55784 [EPIC] Use Parsoid HTML for all page views
Open		None	T258838 [EPIC] Ensure all extensions on the Wikimedia cluster that use Parser hooks or implement tag hooks are retargeted for Parsoid
Open		None	T351374 Improve Translatable Pages based on research input
Open		None	T131516 Reduce or eliminate the need for the user to touch <translate> tags and unit markers
Open		None	T143327 Make the Translate extension be based on DOM, not strings
Resolved		Esanders	T73730 VisualEditor: Content inside templates for translation of templates does not render in friendly ways
Open	Feature	None	T55974 Create a VisualEditor plugin tool to add/edit translations and translation variables (Translate extension)
Resolved		ihurbain	T261172 Much content is missing when editing a translatable page
Resolved		ihurbain	T261181 Make Translate extension compatible with Parsoid
Resolved	BUG REPORT	ihurbain	T295233 PHP Notice: Trying to get property 'parentNode' of non-object

Event Timeline

ssastry created this task.Nov 6 2021, 6:39 PM

ssastry triaged this task as Medium priority.

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptNov 6 2021, 6:39 PM

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

Arlolra moved this task from Needs Triage to Bugs & Crashers on the Parsoid board.Nov 8 2021, 4:15 PM

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

gerritbot added a project: Patch-For-Review.Nov 8 2021, 5:48 PM

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

Still seen on http://meta.wikimedia.org/w/rest.php/meta.wikimedia.org/v3/transform/wikitext/to/pagebundle/Communications%2FCommunicating_about_the_Wikimedia_Foundation (and possibly others)

Maintenance_bot removed a project: Patch-For-Review.Nov 8 2021, 8:11 PM

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>  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

gerritbot added a project: Patch-For-Review.Nov 9 2021, 3:29 PM

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

Maintenance_bot removed a project: Patch-For-Review.Nov 11 2021, 2:10 AM

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

One of them is from some badly nested translate tags in a table which splits up the translate range across table cells which we need to potentially add some robustness support for so we don't crash on the page. | <translate>  rowspan="1" |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> is seen on https://meta.wikimedia.org/wiki/Strategy/Wikimedia_movement/2017/Sources/58_expert_summaries_(June_2017). That should be fixed to move the <translate> tag into the table cell with the content and that will fix the crasher.
https://meta.wikimedia.org/wiki/WikiWomen's_Collaborative is the other page on which we have a crasher. This should be investigated.

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>  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.

ssastry added a parent task: T261181: Make Translate extension compatible with Parsoid.Nov 19 2021, 7:18 PM

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

gerritbot added a project: Patch-For-Review.Nov 29 2021, 6:15 PM

Change 742506 merged by jenkins-bot:

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

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

Maintenance_bot removed a project: Patch-For-Review.Nov 29 2021, 9:10 PM

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