Directory

⚓ T342556 TypeError: Cannot read properties of undefined (reading 'left')
Page MenuHomePhabricator

TypeError: Cannot read properties of undefined (reading 'left')
Closed, ResolvedPublicPRODUCTION ERROR

Description

166 errors in last 7 days
https://logstash.wikimedia.org/app/dashboards#/doc/logstash-*/logstash-default-1-7.0.0-1-2023.07.23?id=-itLg4kBZQQsSJrNFHEj

at PointerLine.setCssProperties  https://ru.m.wikipedia.org/w/load.php?lang=ru&modules=ext.RevisionSlider.Slider%2CdialogImages%2Cinit%7Cjquery.ui%7Cmoment%7Coojs-ui.styles.icons-interactions%2Cicons-moderation&skin=minerva&version=1f546:16:337
at PointerLine.drawLine  https://ru.m.wikipedia.org/w/load.php?lang=ru&modules=ext.RevisionSlider.Slider%2CdialogImages%2Cinit%7Cjquery.ui%7Cmoment%7Coojs-ui.styles.icons-interactions%2Cicons-moderation&skin=minerva&version=1f546:17:434
at SliderView.redrawPointerLines  https://ru.m.wikipedia.org/w/load.php?lang=ru&modules=ext.RevisionSlider.Slider%2CdialogImages%2Cinit%7Cjquery.ui%7Cmoment%7Coojs-ui.styles.icons-interactions%2Cicons-moderation&skin=minerva&version=1f546:42:215
at jQuery.fn.init.<anonymous>  https://ru.m.wikipedia.org/w/load.php?lang=ru&modules=ext.RevisionSlider.Slider%2CdialogImages%2Cinit%7Cjquery.ui%7Cmoment%7Coojs-ui.styles.icons-interactions%2Cicons-moderation&skin=minerva&version=1f546:44:490
at fire  https://ru.m.wikipedia.org/w/load.php?lang=ru&modules=ext.RevisionSlider.Settings%2ClazyJs%7Cext.wikimediaEvents.specialPages%7Cjquery%2Coojs-ui%2Coojs-ui-core%2Coojs-ui-toolbars%2Coojs-ui-widgets%2Coojs-ui-windows%7Cmediawiki.diff%7Coojs-ui-toolbars.icons%7Coojs-ui-widgets.icons%7Coojs-ui-windows.icons%7Coojs-ui.styles.icons-movement&skin=minerva&version=7l5en:50:934
at Object.fireWith [as resolveWith]  https://ru.m.wikipedia.org/w/load.php?lang=ru&modules=ext.RevisionSlider.Settings%2ClazyJs%7Cext.wikimediaEvents.specialPages%7Cjquery%2Coojs-ui%2Coojs-ui-core%2Coojs-ui-toolbars%2Coojs-ui-widgets%2Coojs-ui-windows%7Cmediawiki.diff%7Coojs-ui-toolbars.icons%7Coojs-ui-widgets.icons%7Coojs-ui-windows.icons%7Coojs-ui.styles.icons-movement&skin=minerva&version=7l5en:52:135
at Object.resolve  https://ru.m.wikipedia.org/w/load.php?lang=ru&modules=ext.RevisionSlider.Settings%2ClazyJs%7Cext.wikimediaEvents.specialPages%7Cjquery%2Coojs-ui%2Coojs-ui-core%2Coojs-ui-toolbars%2Coojs-ui-widgets%2Coojs-ui-windows%7Cmediawiki.diff%7Coojs-ui-toolbars.icons%7Coojs-ui-widgets.icons%7Coojs-ui-windows.icons%7Coojs-ui.styles.icons-movement&skin=minerva&version=7l5en:63:331
at fire  https://ru.m.wikipedia.org/w/load.php?lang=ru&modules=ext.RevisionSlider.Settings%2ClazyJs%7Cext.wikimediaEvents.specialPages%7Cjquery%2Coojs-ui%2Coojs-ui-core%2Coojs-ui-toolbars%2Coojs-ui-widgets%2Coojs-ui-windows%7Cmediawiki.diff%7Coojs-ui-toolbars.icons%7Coojs-ui-widgets.icons%7Coojs-ui-windows.icons%7Coojs-ui.styles.icons-movement&skin=minerva&version=7l5en:50:934
at Object.fireWith  https://ru.m.wikipedia.org/w/load.php?lang=ru&modules=ext.RevisionSlider.Settings%2ClazyJs%7Cext.wikimediaEvents.specialPages%7Cjquery%2Coojs-ui%2Coojs-ui-core%2Coojs-ui-toolbars%2Coojs-ui-widgets%2Coojs-ui-windows%7Cmediawiki.diff%7Coojs-ui-toolbars.icons%7Coojs-ui-widgets.icons%7Coojs-ui-windows.icons%7Coojs-ui.styles.icons-movement&skin=minerva&version=7l5en:52:135
at Object.fire  https://ru.m.wikipedia.org/w/load.php?lang=ru&modules=ext.RevisionSlider.Settings%2ClazyJs%7Cext.wikimediaEvents.specialPages%7Cjquery%2Coojs-ui%2Coojs-ui-core%2Coojs-ui-toolbars%2Coojs-ui-widgets%2Coojs-ui-windows%7Cmediawiki.diff%7Coojs-ui-toolbars.icons%7Coojs-ui-widgets.icons%7Coojs-ui-windows.icons%7Coojs-ui.styles.icons-movement&skin=minerva&version=7l5en:52:179

Event Timeline

thiemowmde moved this task from Incoming to Maintenance on the Revision-Slider board.
thiemowmde changed the subtype of this task from "Bug Report" to "Production Error".

@Jdlrobson, I'm afraid I need your help. These logstash entries don't make sense for a specific reason: They all happen on some mobile domain (e.g. https://uk.m.wikipedia.org), all with the same URL scheme that points to action=diff, e.g. …/w/index.php?diff=…&oldid=…. My problem is: No matter what I do, I'm entirely unable to access these URLs. I immediately get redirected to Special:MobileDiff. And RevisionSlider is disabled on MobileDiff.

What's going on here? It looks like these users somehow manage to stay on the regular action=diff without being redirected.

Perhaps a gadget or other extension is pulling it in or there is a bug in the redirect code?

Regardless given any code ''can'' be loaded on mobile or desktop now I think this code needs to be rewritten in a more defensive way so that it doesn't error. Return early from the code.

Note you can also log your own custom error to your own error channel if you want to understand better why it's happening (example: https://github.com/wikimedia/mediawiki-extensions-Popups/blob/master/src/index.js#L159)

thiemowmde moved this task from Doing to Backlog on the WMDE-TechWish-Maintenance-2023 board.
thiemowmde subscribed.

Change 968226 had a related patch set uploaded (by WMDE-Fisch; author: WMDE-Fisch):

[mediawiki/extensions/RevisionSlider@master] Make sure to not load on the mobile view

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

Change 968229 had a related patch set uploaded (by WMDE-Fisch; author: WMDE-Fisch):

[mediawiki/extensions/RevisionSlider@master] Only draw lines connecting the diff when it's availible

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

Change 968229 merged by jenkins-bot:

[mediawiki/extensions/RevisionSlider@master] Only draw lines connecting the diff when it's availible

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

I tried to dig a bit deeper into the code. Some findings so far:

  • It appears like it should be possible to disable Special:MobileDiff via $wgMFUseDesktopDiffPage. However, it doesn't look like this is in use.
  • There is apparently a mobile-specialpages user preference the user can choose to disable. This is behind a $wgMFEnableMobilePreferences feature flag. But it's very much possible some users had access to this at one point and still have it disabled.
  • However, it doesn't look like this can trigger any error. When I try this locally I can use RevisionSlider in the mobile view with no problems. It's just not compatible with Special:MobileDiff.
  • I found a bigger chunk of duplication in the MobileFrontend codebase that leaves me somewhat confused. It looks like MobileFrontendHooks::onDifferenceEngineViewHeader() and MobileContext::redirectMobileEnabledPages() do more or less the same things (redirecting to Special:MobileDiff) the same time, possibly even fighting with each other. One of the pieces was added via Ice81bc3 in 2014, the other via Id0c4b9b in 2013.

Change 968226 abandoned by WMDE-Fisch:

[mediawiki/extensions/RevisionSlider@master] Make sure to not load on the mobile view

Reason:

For the reasons mentioned in https://phabricator.wikimedia.org/T342556#9275526 - It seems it's possible to have the mobile view and a diff page working with the RevisionSlider.

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

Waiting for the fixes to get deployed to validate the error is gone.

FYI I'm hoping Special:MobileDiff will be obsolete within a year. There's a patch for T240624 which is the last remaining blocker for deployment to smaller wikis.

WMDE-Fisch moved this task from Review to Done on the WMDE-TechWish-Maintenance-2023 board.
WMDE-Fisch subscribed.

Now that the current patch is deployed, I looked at logstash to validate that the issue is gone. But it is not. It also seems to be unrelated to mobile because there's for example one occurrence on a page like [1]. With the following trace:

at PointerLine.setCssProperties  https://uz.wikipedia.org/w/load.php?lang=uz&modules=ext.RevisionSlider.Slider&skin=vector&version=vgpsi:17:664
at PointerLine.drawLine  https://uz.wikipedia.org/w/load.php?lang=uz&modules=ext.RevisionSlider.Slider&skin=vector&version=vgpsi:18:463
at SliderView.redrawPointerLines  https://uz.wikipedia.org/w/load.php?lang=uz&modules=ext.RevisionSlider.Slider&skin=vector&version=vgpsi:44:438
at jQuery.fn.init.eval  https://uz.wikipedia.org/w/load.php?lang=uz&modules=ext.RevisionSlider.Slider&skin=vector&version=vgpsi:46:547
at fire  https://uz.wikipedia.org/w/load.php?lang=uz&modules=jquery%2Coojs-ui-core%2Coojs-ui-widgets%2Cvue%7Cjquery.ui%7Cwikibase.client.linkitem.init&skin=vector&version=19l6p:42:705
at Object.fireWith [as resolveWith]  https://uz.wikipedia.org/w/load.php?lang=uz&modules=jquery%2Coojs-ui-core%2Coojs-ui-widgets%2Cvue%7Cjquery.ui%7Cwikibase.client.linkitem.init&skin=vector&version=19l6p:43:903
at Object.resolve  https://uz.wikipedia.org/w/load.php?lang=uz&modules=jquery%2Coojs-ui-core%2Coojs-ui-widgets%2Cvue%7Cjquery.ui%7Cwikibase.client.linkitem.init&skin=vector&version=19l6p:55:172
at fire  https://uz.wikipedia.org/w/load.php?lang=uz&modules=jquery%2Coojs-ui-core%2Coojs-ui-widgets%2Cvue%7Cjquery.ui%7Cwikibase.client.linkitem.init&skin=vector&version=19l6p:42:705
at Object.fireWith  https://uz.wikipedia.org/w/load.php?lang=uz&modules=jquery%2Coojs-ui-core%2Coojs-ui-widgets%2Cvue%7Cjquery.ui%7Cwikibase.client.linkitem.init&skin=vector&version=19l6p:43:903
at Object.fire  https://uz.wikipedia.org/w/load.php?lang=uz&modules=jquery%2Coojs-ui-core%2Coojs-ui-widgets%2Cvue%7Cjquery.ui%7Cwikibase.client.linkitem.init&skin=vector&version=19l6p:43:947

Further looking into that trace and the exact line and column where the issue appears it seems that - at least in that case above - it's not about the missing $targetColumn where we can't get the .offset().left but it's $( '.mw-revslider-revision-slider' ).offset().left;. - I guess there's a weird race condition when the slider is not fully initialized but that redraw event is fired....

[1] https://uz.wikipedia.org/w/index.php?diff=3931700&oldid=3931693&title=Temurbek_Poyonov&curid=940895

Change 971398 had a related patch set uploaded (by WMDE-Fisch; author: WMDE-Fisch):

[mediawiki/extensions/RevisionSlider@master] Don't try to draw lines when the slider is not fully loaded

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

Change 971398 merged by jenkins-bot:

[mediawiki/extensions/RevisionSlider@master] Don't try to draw lines when the slider is not fully loaded

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

Change 972049 had a related patch set uploaded (by Thiemo Kreuz (WMDE); author: Thiemo Kreuz (WMDE)):

[mediawiki/extensions/RevisionSlider@master] Remove redundant checks for `.offset() !== undefined`

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

Change 972049 merged by jenkins-bot:

[mediawiki/extensions/RevisionSlider@master] Remove redundant checks for `.offset() !== undefined`

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

You can see dropping numbers on Logstash now with 1.42.0-wmf.4 being rolled out.

Tobi_WMDE_SW claimed this task.