Page MenuHomePhabricator

Misleading message shows in skins where VE is compatible but the page because of its state isn't ["Incompatible skin" / "Incompatible with VisualEditor"]
Closed, ResolvedPublicBUG REPORT

Description

Steps to replicate the issue (include links if applicable):

What happens?:

What should have happened instead?:

  • The skin is not the issue here so a more appropriate warnign should show e.g. File pages are not compatible with VisualEditor.

Software version (skip for WMF-hosted wikis like Wikipedia):

Other information (browser name/version, screenshots, etc.):

Event Timeline

Presence of the edit tab in SkinTemplate is conditional on Title::canExist() + Authority::probablyCan('read') + Authority::probablyCan('edit'). I think the read check is superfluous and only done as a performance optimization. The edit check is available on the JS side as mw.config.get('wgIsProbablyEditable'). The canExist check at a glance is not done by the permission system, and I don't think it is exposed via JS. But it's probably a pretty rare case - it only happens for some invalid titles, and I don't see the error there. Presumably the pageCanLoadEditor check already covers that.

Change 822207 had a related patch set uploaded (by Gergő Tisza; author: Gergő Tisza):

[mediawiki/extensions/VisualEditor@master] Do not show incompatible skin warning when page is not editable

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

Change 822207 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Do not show incompatible skin warning when page is not editable

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

Change 822396 had a related patch set uploaded (by Jdlrobson; author: Gergő Tisza):

[mediawiki/extensions/VisualEditor@wmf/1.39.0-wmf.24] Do not show incompatible skin warning when page is not editable

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

Change 822402 had a related patch set uploaded (by Thcipriani; author: Gergő Tisza):

[mediawiki/extensions/VisualEditor@wmf/1.39.0-wmf.23] Do not show incompatible skin warning when page is not editable

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

Change 822402 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@wmf/1.39.0-wmf.23] Do not show incompatible skin warning when page is not editable

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

Mentioned in SAL (#wikimedia-operations) [2022-08-11T20:29:49Z] <thcipriani@deploy1002> Synchronized php-1.39.0-wmf.23/extensions/VisualEditor/modules/ve-mw/preinit/ve.init.mw.DesktopArticleTarget.init.js: Backport: [[gerrit:822396|Do not show incompatible skin warning when page is not editable (T314952)]] (duration: 03m 16s)

Change 822396 abandoned by Jdlrobson:

[mediawiki/extensions/VisualEditor@wmf/1.39.0-wmf.24] Do not show incompatible skin warning when page is not editable

Reason:

There is no wmf24

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

I backported the patch and it does seem like that may have addressed the issue:

Screen Shot 2022-08-11 at 9.52.26 PM.png (710×2 px, 150 KB)

Screen Shot 2022-08-11 at 9.53.06 PM.png (336×2 px, 61 KB)

Will monitor a little longer.

Please feel free to resolve once we have 12 hrs of data.

Jdlrobson claimed this task.
This comment was removed by Jdlrobson.

5508 errors in last 24hrs: https://logstash.wikimedia.org/goto/80d3ba555b0788b16f7ba05e3cb410eb
Unclear if this is caching related or evidence of a remaining problem.

Jdlrobson renamed this task from Misleading message shows in skins where VE is compatible but the page because of its state isn't to Misleading message shows in skins where VE is compatible but the page because of its state isn't [Incompatible skin: vector(-2022)].Aug 15 2022, 2:59 PM

At a glance it is now happening on pages where there is no reason for it to happen.

Assets are only cached for a few minutes; Varnish caches HTML for one week I think, and we are well past that since the last train deploy, so I don't think it's caching related.

Can we change the error message slightly to prove/disprove that the errors are only being logged by cached old code?

(While we're there, we could add some more details about how the skin is incompatible, i.e. which elements are missing.)

Change 825293 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/extensions/VisualEditor@master] Error logging: Provide additional debugging context

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

Change 825293 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Error logging: Provide additional debugging context

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

matmarex renamed this task from Misleading message shows in skins where VE is compatible but the page because of its state isn't [Incompatible skin: vector(-2022)] to Misleading message shows in skins where VE is compatible but the page because of its state isn't ["Incompatible skin" / "Incompatible with VisualEditor"].Aug 30 2022, 4:33 PM
matmarex added a subscriber: bwang.

rEVEDc697a6d2dac1: Do not show incompatible skin warning when page is not editable has been in production for two weeks and rEVEDe763f9d2f259: Error logging: Provide additional debugging context for one week so re-checking.

Seems to produce about 30-40 errors per hour at peak time, with Incompatible with VisualEditor: 1-0-1-1-0 which means init.isVisualAvailable, pageCanLoadEditor and pageIsProbablyEditable are true, init.isWikitextAvailable and requiredSkinElements are false. Seems to come from a fairly random assortment of wikis, and I don't see anything special about the pages on which it happens. Could it be some kind of race condition with the check running before the skin element is reliably there? Or some very widely used gagdet?

var requiredSkinElements =
                        $(
                          document.querySelector( '[data-mw-ve-target-container]' ) ||
                          document.getElementById( 'content' )
                        ).length &&
                        $( '#mw-content-text' ).length &&
                        $( '#ca-edit, #ca-viewsource' ).length

The most likely thing is ca-edit or ca-viewsource are missing from the page due to a gadget/site script.

Is there any circumstances where these wouldn't be present? Maybe something like https://bar.wikipedia.org/wiki/MediaWiki:Gadget-HideVisualEditor.js , https://en.wikipedia.org/wiki/User:Yair_rand/UserBlind.js?

If it helps we could update the error logging to provide further information, or perhaps we want to restrict logging to anonymous users (since there's not much we can do about logged in users running gadgets here)?

I've been looking at the error on the mediawiki-client-errors dashboard (https://logstash.wikimedia.org/goto/266fe439d8e22cc770a4595ab972f119) and it looks like some wikis are overrepresented (ja.wp and pt.wp in particular), and also errors on Firefox are overrepresented. I've been browsing around for some gadget or browser extension that might be responsible, but unsuccessfully.

We should probably just improve JS error logging in general. Log the skin, whether the user is logged in, maybe the list of active gadgets or RL modules (although there are privacy aspects to that).

Change 829230 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/extensions/VisualEditor@master] Run DesktopArticleTarget on jQuery ready

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

I was able to replicate this after hitting the random access key a few times. THe issue seems to be the fact targetContainer is calculated outside document.ready, so isn't available at the time of check:

// before:

( function () {
   var $targetContainer = $(
			document.querySelector( '[data-mw-ve-target-container]' ) ||
			document.getElementById( 'content' )
		);
}());

//after

$( function () {
   var $targetContainer = $(
			document.querySelector( '[data-mw-ve-target-container]' ) ||
			document.getElementById( 'content' )
		);
});

That looks right! It's been so long since I ran into a problem like this, I forgot it's possible. Caused by https://gerrit.wikimedia.org/r/c/mediawiki/extensions/VisualEditor/+/807196.

The fix is slightly more involved – there's already a $( function () { … } ) in that file, a bit further down: https://gerrit.wikimedia.org/g/mediawiki/extensions/VisualEditor/+/0ade4be5b62dd6ad0584cc7e0c75e473f65447d2/modules/ve-mw/preinit/ve.init.mw.DesktopArticleTarget.init.js#1510

This is because the file also defines the mw.libs.ve API: https://gerrit.wikimedia.org/g/mediawiki/extensions/VisualEditor/+/0ade4be5b62dd6ad0584cc7e0c75e473f65447d2/modules/ve-mw/preinit/ve.init.mw.DesktopArticleTarget.init.js#1463 which must happen before document-ready to support use cases like this: https://www.mediawiki.org/wiki/VisualEditor/Gadgets#User_script

So a bunch of stuff needs to be moved around. I'll update the patch.

Change 829230 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] DesktopArticleTarget.init: Get $targetContainer after jQuery ready

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

This still appears to be happening unfortunately.

It dropped significantly after the patch:

image.png (280×3 px, 38 KB)

…but not low enough that we could just say "caching". I guess there's another issue.

I was possibly looking at wrong query. The bulk of these errors seem to have vanished now? Only 5 errors in last 3 days which is nothing:

Screen Shot 2022-10-03 at 5.58.56 PM.png (367×1 px, 54 KB)

I think we can actually resolve this now?

Actually..something looks very wrong
https://grafana.wikimedia.org/d/000000566/overview?orgId=1&viewPanel=16
We either fixed a bunch of bugs or made some change to the error logging pipeline.
I've opened up https://phabricator.wikimedia.org/T319261 to investigate first.

Change 848556 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/extensions/VisualEditor@master] Add further debugging information

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

Still occurring. I'd like to add some additional debugging code to verify that we're not working with broken HTML.

Change 848556 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Add further debugging information

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

Change 849637 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/extensions/VisualEditor@master] wgAction is not a number

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

According to data from Italian Wikipedia, impacted pages do not have any edit link $( '#ca-edit, #ca-viewsource' ).length, but have all the required other elements. It's unclear what wgAction is right now due to a bug in my debugging information patch, but that's likely 'view'

So the question is.. why would a page not have a #ca-edit, #ca-viewsource link? Is something removing it before this code runs?

It's unclear what wgAction is right now due to a bug in my debugging information patch

We are now logging it under error_context.action, FWIW.
(Also the logged-in status, which shows that these are largely anonymous pageviews.)

Change 849637 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] wgAction is not a number

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

Change 852309 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/extensions/VisualEditor@master] Don't log errors due to missing edit buttons

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

Change 852309 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Don't log errors due to missing edit buttons

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

I guess we can close this too. The mystery remains unsolved, but we're not logging the errors any more.