Page MenuHomePhabricator

Fundraising banner with inline SVG triggers `Uncaught TypeError: node.className.replace is not a function` for layout shift due to SVGAnimatedString className attribute.
Closed, ResolvedPublic

Description

This might relate to T276825 in some way, since there is a banner running and that would emitLayoutShift. Error is impacting multiple IP addresses mostly on Wikimedia commons https://logstash.wikimedia.org/app/dashboards#/doc/logstash-*/logstash-2021.03.08?id=Q86hEngBfVMx58vqJ3xD

at URL1:694:36
at emitLayoutShift  URL1:693:465
at PerformanceObserver.<anonymous>  URL1:694:357

URL1: https://commons.wikimedia.org/w/load.php?lang=en&modules=dataValues%2Cjquery%2Coojs%2Coojs-router%2Coojs-ui%2Coojs-ui-core%2Coojs-ui-toolbars%2Coojs-ui-widgets%2Coojs-ui-windows%2Csite%2CvalueFormatters%2CvalueParsers%2Cvue%2Cwikibase%7CdataValues.DataValue%2CTimeValue%2Cvalues%7Cext.centralNotice.choiceData%2Cdisplay%2CgeoIP%2CimpressionDiet%2CkvStore%2CstartUp%7Cext.centralauth.ForeignApi%2Ccentralautologin%7Cext.eventLogging%2Ckartographer%2CnavigationTiming%2CwikimediaEvents%7Cext.kartographer.link%2Clinkbox%2Cutil%7Cext.uls.common%2Ccompactlinks%2Ci18n%2Cinterface%2Clanguagenames%2Cmediawiki%2Cmessages%2Cpreferences%2Cwebfonts%7Cext.wikimediaEvents.wikibase%7Cjquery.animateWithEvent%2Cclient%2Ccookie%2Ci18n%2Cinputautoexpand%2CtextSelection%2Ctipsy%2Cui%2Culs%2Cvalueview%7Cjquery.event.special.eachchange%7Cjquery.ui.commonssuggester%2Clanguagesuggester%2Csuggester%7Cjquery.uls.data%2Cgrid%7Cjquery.util.getDirectionality%7Cjquery.valueview.Expert%2CExpertExtender%2CExpertStore%7Cjquery.valueview.experts.CommonsMediaType%2CEmptyValue%2CGeoShape%2CGlobeCoordinateInput%2CMonolingualText%2CQuantityInput%2CStringValue%2CTabularData%2CTimeInput%2CUnDeserializableValue%7Cjquery.wikibase.entityselector%2Cwbtooltip%7Cmediawiki.ForeignApi%2CString%2CTitle%2CUri%2Capi%2Cbase%2Ccldr%2CconfirmCloseWindow%2Ccookie%2Cexperiments%2CjqueryMsg%2Clanguage%2Crouter%2Cstorage%2Ctemplate%2Cuser%2Cutil%7Cmediawiki.ForeignApi.core%7Cmediawiki.action.edit.editWarning%7Cmediawiki.action.view.metadata%7Cmediawiki.editfont.styles%7Cmediawiki.libs.pluralruleparser%7Cmediawiki.page.ready%7Cmediawiki.page.watch.ajax%7Cmediawiki.template.mustache%2Cmustache%2Bdom%7Cmediawiki.ui.button%2Cicon%7Cmmv.bootstrap%2Chead%7Cmmv.bootstrap.autostart%7Cmw.config.values.wbDataTypes%2CwbRefTabsEnabled%2CwbRepo%2CwbSiteDetails%7Coojs-ui-core.icons%2Cstyles%7Coojs-ui-toolbars.icons%7Coojs-ui-widgets.icons%7Coojs-ui-windows.icons%7Coojs-ui.styles.icons-editing-core%2Cicons-interactions%2Cicons-location%2Cicons-movement%2Cindicators%7Cskins.vector.legacy.js%7Cuser.defaults%7Cutil.ContentLanguages%2CExtendable%2CMessageProvider%2CMessageProviders%2CNotifier%2ChighlightSubstring%2Cinherit%7CvalueParsers.ValueParserStore%2Cparsers%7Cwikibase.EntityInitializer%2CSite%2CWikibaseContentLanguages%2CbuildErrorOutput%2Cdatamodel%2CgetLanguageNameByCode%2CgetUserLanguages%2Cserialization%2Csites%2Ctainted-ref%2Ctemplates%7Cwikibase.api.RepoApi%2CValueCaller%7Cwikibase.entityChangers.EntityChangersFactory%7Cwikibase.entityPage.entityLoaded%7Cwikibase.experts.Entity%2CItem%2CProperty%2C__namespace%2Cmodules%7Cwikibase.formatters.ApiValueFormatter%7Cwikibase.mediainfo.base%2CfilePageDisplay%2Csearch%2Cstatements%2Culs%7Cwikibase.ui.entityViewInit%7Cwikibase.utilities.ClaimGuidGenerator%7Cwikibase.view.ControllerViewFactory%2CReadModeViewFactory%2C__namespace&skin=vector&version=yqcjx

`

Event Timeline

Gilles triaged this task as High priority.
Gilles moved this task from Inbox to Doing: Prio Interrupt on the Performance-Team board.
if ( 'className' in node ) {
	event.firstSourceNode = event.firstSourceNode + '.' + node.className.replace( /\s/g, '.' );
}

If this fails, it seems one of two things must be happening:

  1. The browser is completely broken and has an HTMLElement subclass in which the className property exists as a non-string (extremely unlikely).
  2. Some javascript code has modified String.prototype and deleted the replace() method for everyone?

(Note that className as property is not the same as class attribute, the property always exists with an empty string default.)

This comment was removed by Jdlrobson.

If this fails, it seems one of two things must be happening:

Or node is not a DOMElement?

I can replicate this issue now.

According to https://developer.mozilla.org/en-US/docs/Web/API/Element/className "className can also be an instance of SVGAnimatedString if the element is an SVGElement". This attribute is deprecated and may be removed in a future version of this specification. Authors are advised to use Element.classList instead. The SVGAnimatedString interface do not provide any specific methods.

That is the case with the International Womans day banner which uses an svg element for its close icon. When I select it className is not a string.

document.querySelectorAll('.frb-inline-close svg')[0].className.replace is not a function.

I've dumped the HTML of the banner for prosperity so you can verify the above line in Chrome.
https://gist.github.com/jdlrobson/ec0868efd8472ac91042caaba54e60e8

Jdlrobson renamed this task from Uncaught TypeError: node.className.replace is not a function to Fundraising banner with inline SVG triggers `Uncaught TypeError: node.className.replace is not a function` for layout shift due to SVGAnimatedString className attribute..Mar 8 2021, 10:34 PM

Screen Shot 2021-03-08 at 2.36.02 PM.png (1×2 px, 501 KB)

If this fails, it seems one of two things must be happening:

Or node is not a DOMElement?

I can replicate this issue now.

According to https://developer.mozilla.org/en-US/docs/Web/API/Element/className "className can also be an instance of SVGAnimatedString if the element is an SVGElement".

SVGElement is a subclass of the Element class. However, it indeed has a historical quick that changes className to be something else. Never seen this before. Interesting. TIL 🙂

Change 670126 had a related patch set uploaded (by Gilles; owner: Gilles):
[mediawiki/extensions/NavigationTiming@master] Fix layout shift class name parsing for SVGElement

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

Change 670126 merged by jenkins-bot:
[mediawiki/extensions/NavigationTiming@master] Fix layout shift class name parsing for SVGElement

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

Change 670125 had a related patch set uploaded (by Krinkle; owner: Gilles):
[mediawiki/extensions/NavigationTiming@wmf/1.36.0-wmf.34] Fix layout shift class name parsing for SVGElement

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

Change 670125 merged by jenkins-bot:
[mediawiki/extensions/NavigationTiming@wmf/1.36.0-wmf.34] Fix layout shift class name parsing for SVGElement

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

Mentioned in SAL (#wikimedia-operations) [2021-03-10T01:08:25Z] <krinkle@deploy1002> Synchronized php-1.36.0-wmf.34/extensions/NavigationTiming/modules/ext.navigationTiming.js: T276826 Ibd9ddf14d64 (duration: 01m 14s)