Page MenuHomePhabricator

Edit buttons overlap on Minerva on Wikidata
Open, Needs TriagePublicBUG REPORT

Description

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

What happens?:
You have an overlay of buttons to edit the page. Therefore it is impossible to click on the good one.

What should have happened instead?:
Edit buttons should be clearly separated.

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

Other information (browser name/version, screenshots, etc.):
I use Firefox on Android.

Screenshot_20230630-141712-01.jpeg (1×1 px, 130 KB)

Event Timeline

I can reproduce it, but only while logged in, and not with ?safemode=1.

One of the links seems to be the “Add a new statement” button, so my guess is that the Wikibase desktop editing interface is being inadvertently pulled into the mobile view by some gadget, now that it’s no longer guarded by the targets system (T324991: Make Wikibase not rely on the ResourceLoader target system).

Yup, if I disable AuthorityControl and DuplicateReferences in my preferences then the issue goes away. (I expect those aren’t the only affected gadgets, but it’ll depend on which gadgets any user has enabled or disabled.)

I guess the affected gadgets need to be edited to load the dependencies dynamically in their JS code, rather than declare them in MediaWiki:Gadgets-definition, and also skip loading on mobile (mw.config.get( 'wgMFMode' ) !== null)? Not sure.

I've removed most of my gadgets and scripts but the problem persists.

The mobile site is so limited that it's not really usable anyway so my recommendation would be to disable it on Wikidata and focus on improving the desktop site instead (see T330820)

I personally have zero motivation to try to fix the gadgets when I didn't break them, I don't see how I benefit from the changes that broke them, I don't know which ones are broken and I don't really understand why they're broken (beyond "somehow something loaded something it shouldn't have"). You might have more luck with another interface admin, but maybe it would be better for the desktop-only Wikibase code to refuse to run if it gets loaded on mobile, instead of trying to make sure nobody ever loads it on mobile?

I guess the affected gadgets need to be edited to load the dependencies dynamically in their JS code, rather than declare them in MediaWiki:Gadgets-definition, and also skip loading on mobile (mw.config.get( 'wgMFMode' ) !== null)? Not sure.

There is no good technical reason for anything to not be usable on mobile. Typically this occurs because gadget developers are not using the standardized APIs in https://www.mediawiki.org/wiki/ResourceLoader/Core_modules or that API hasn't yet been flagged to the core developers and thus doesn't exist yet.

Yup, if I disable AuthorityControl and DuplicateReferences in my preferences then the issue goes away.

What are those gadgets trying to do? Where are they trying to add to and are they using mw.util.addPortletLink ? Perhaps T336431 may be of help?

maybe it would be better for the desktop-only Wikibase code to refuse to run if it gets loaded on mobile, instead of trying to make sure nobody ever loads it on mobile?

That’s what we had with the targets system, until it was declared deprecated and removed 🤷 but it’s not really possible to do such a check inside Wikibase’s JS code, since the visual issue in this task is caused by the CSS stylesheets, which ResourceLoader adds directly.

Jdlrobson edited projects, added MinervaNeue (Tracking); removed MinervaNeue.

@Lucas_Werkmeister_WMDE this is a bit off topic but have we ever considered dropping MobileFrontend and Minerva from Wikidata.org and instead enable wgVectorResponsive? The main benefit of MobileFrontend these days is content massaging (e.g. section collapsing, lazy loaded images and removal of navboxes). I don't think Wikidata uses any of that and MobileFrontend is very Wikipedia content centric so using MobileFrontend seems to create more issues than solve them (lots of features don't work on mobile as it stands).

TBH most of what MobileFrontend does is undone by Wikidata at this point and it might be better strategically to invest in and improve the desktop code to be more mobile friendly using a responsive version of Vector 2022. Let me know if this resonates and I can create a ticket.

@Jdlrobson removing MinervaNeue for Wikidata would be an option. MinervaNeue is very limited in editing item pages. However, MinervaNeue is very useful to read and edit all other pages such as talk pages, user pages, pages in the Wikidata and Help namespaces.

I contribute mostly from my mobile phone and I appreciate using MinervaNeue for a large part of edits.

Tacsipacsi renamed this task from Bug with the Minerva interface for Wikidata to Edit buttons overlap on Minerva on Wikidata.Jul 20 2023, 9:10 AM
Tacsipacsi subscribed.

@PAC2 Minerva is not the problem here. I use the desktop skin on Wikidata.org without any problems: e.g. https://www.wikidata.org/wiki/Q42394599?useskin=minerva The problem is MobileFrontend which makes skins behave differently.
Take a look at Vector 2022 for example which would be even more broken: https://m.wikidata.org/wiki/Q42394599?useskin=vector-2022

The styles https://www.wikidata.org/wiki/User:Jdlrobson/minerva.css if applied would improve things on desktop and Minerva.

@Lucas_Werkmeister_WMDE this is a bit off topic but have we ever considered dropping MobileFrontend and Minerva from Wikidata.org and instead enable wgVectorResponsive? The main benefit of MobileFrontend these days is content massaging (e.g. section collapsing, lazy loaded images and removal of navboxes). I don't think Wikidata uses any of that and MobileFrontend is very Wikipedia content centric so using MobileFrontend seems to create more issues than solve them (lots of features don't work on mobile as it stands).

TBH most of what MobileFrontend does is undone by Wikidata at this point and it might be better strategically to invest in and improve the desktop code to be more mobile friendly using a responsive version of Vector 2022. Let me know if this resonates and I can create a ticket.

So far no. Would you mind opening a ticket? I think it's worth discussing.

I guess the affected gadgets need to be edited to load the dependencies dynamically in their JS code, rather than declare them in MediaWiki:Gadgets-definition, and also skip loading on mobile (mw.config.get( 'wgMFMode' ) !== null)? Not sure.

FWIW, this is still the only solution that I see. The Wikibase desktop-only modules are not suitable for loading on mobile; we went through considerable effort in Wikibase to avoid loading them despite the targets system being removed; gadgets now have to make a comparable effort. (I still think that the removal of the targets system was a grave mistake, but I don’t think it’s going to come back, so we’re just all going to have to live with that, I’m afraid.)

Suggested edit for AuthorityControl:

diff --git a/Gadget-AuthorityControl.js b/Gadget-AuthorityControl.js
index 6047d4ad71..80835c04f5 100644
--- a/Gadget-AuthorityControl.js
+++ b/Gadget-AuthorityControl.js
@@ -12,6 +12,11 @@ if ( [ 0, 120, 146 ].indexOf( mw.config.get( 'wgNamespaceNumber' ) ) === -1 || !
 	return;
 }
 
+if ( mw.config.get( 'wgMFMode' ) !== null ) {
+	// Mobile interface not supported (T340859).
+	return;
+}
+
 var PROPERTIES = {},
 	specialHandlingProperties = [
 		'P426', // aircraft registration
@@ -360,19 +365,23 @@ function getProperties( entity ) {
 }
 
 var rendered = $.Deferred(),
-	loaded = $.Deferred();
+	loaded = $.Deferred(),
+	dependencies = mw.loader.using( [ 'wikibase.api.RepoApi', 'wikibase.view.ControllerViewFactory' ] );
 
 $.when(
 	rendered,
 	loaded,
+	dependencies,
 	$.ready
 ).then( function () {
 	initGadget();
 } );
 
 mw.hook( 'wikibase.entityPage.entityLoaded' ).add( function ( entity ) {
-	getProperties( entity ).then( function () {
-		loaded.resolve();
+	dependencies.then( function () {
+		getProperties( entity ).then( function () {
+			loaded.resolve();
+		} );
 	} );
 } );

(The loading of dependencies is a bit awkward. Arguably, it could be removed from the outer $.when() call; it’s definitely needed before getProperties(), though, because getProperties() uses wb.api.RepoApi.)

Relying on wgMFMode is not a good idea as that could disappear in future. Ideally you'd feature detect something in the UI or the browser state (e.g. checking viewport size). Per the frontend stable policy you should request new APIs if what you need to feature detect is not possible to feature detect.

You can currently test if MobileFrontend code is running using mw.loader.getState( 'mobile.init' ) !== 'registered' but it is better to feature detect for the precise thing you want e.g. "If I click the edit button will the mobile editor show?" . That's going to be more robust and stable code in the long run.

Off topic: Regarding "removal of the targets system was a grave mistake" you've said this a few times now in various tickets, and it's possible that you might have some misunderstandings of what the targets system was doing, how it worked and why it was introduced in the first place. Let me know if it would be useful to have a chat and/or if some kind of document would be useful to help clarify with why we're doing this and what the future looks like. On the longer term, I see MobileFrontend disappearing altogether and us moving towards two responsive skins - one which is mobile first (Minerva) and one which is desktop first (Vector 2022). Gadgets and extensions will need to prepare for this. I wrote down a few reflections a while back on the relationship between MobileFrontend and Minerva which may also be useful context.

it is better to feature detect for the precise thing you want e.g. "If I click the edit button will the mobile editor show?" . That's going to be more robust and stable code in the long run.

The question is not “if I click the edit button will the mobile editor show?”; the question is “is the ResourceLoader module I’m about to load compatible with the environment I’m using?”. Before the removal of the targets system, it was the same question as “does the module support the target I’m using?”, and if the answer was “no”, ResourceLoader refused to load the module. Since 74be4c581a7, it’s the same question as “is the WikibaseRepo.MobileSite ’service’ [against whose widepread use I unsuccessfully argued in the code review of 554077775ff] true?” or “does MobileContext::shouldDisplayMobileView() return true?”. Neither of these have direct JavaScript equivalents, the closest equivalent being whether there is any variable present which is defined only if MobileContext::shouldDisplayMobileView() is true.

In the long run, we don’t need a robust solution for determining whether a module will mess up the page when loaded, but modules that simply don’t mess up the page.

By the way, why isn’t mw.config.get('wgMFMode') a stable interface? MobileFrontend sets this variable, but never uses it, so it’s clearly there for the benefit of third-party users like gadget authors.

Ideally you'd feature detect something in the UI or the browser state (e.g. checking viewport size).

Viewport size or any other client-side state is irrelevant in this case. The problem is that gadgets must not load Wikibase ResourceLoader modules that aren’t intended for the currently loaded view.

I can’t actually reproduce the issue with AuthorityControl anymore, so the patch in T340859#9239476 can probably be ignored. In DuplicateReferences (the other gadget mentioned in T340859#8981437), I think removing the dependency on wikibase.ui.entityViewInit in the gadgets-definition is enough to solve this issue; the DuplicateReferences code itself then needs to be tweaked a bit further to avoid an error when wb.EntityInitializer isn’t defined.

Since 74be4c581a7, it’s the same question as “is the WikibaseRepo.MobileSite ’service’ [against whose widepread use I unsuccessfully argued in the code review of 554077775ff] true?” or “does MobileContext::shouldDisplayMobileView() return true?”. Neither of these have direct JavaScript equivalents, the closest equivalent being whether there is any variable present which is defined only if MobileContext::shouldDisplayMobileView() is true.

I think the correct question for gadgets to ask is actually “did Wikibase add the ResourceLoader modules or not?” (And Wikibase would’ve done that based on those PHP methods that don’t have direct JS equivalents, but the gadgets don’t need to care about that in detail.) So, something like:

if ( ![ 'loading', 'loaded', 'executing', 'ready' ].includes( mw.loader.getState( 'wikibase.ui.entityViewInit' ) ) ) {
    // no Wikibase entity view, skip
    return;
}

What do you think? (Array.includes() is included in MediaWiki’s required JS version now AFAIK.)

@Tacsipacsi mw.config variables are not currently covered by the frontend stable policy. FWIW I originally added that variable for internal usage in MobileFrontend back in 2013 when mobile had a beta mode and MobileFrontend didn't use ResourceLoader. As you point out it's no longer used internally, but it's adoption in WikimediaEvents and various other extensions has made it slow to remove.

@Lucas_Werkmeister_WMDE using mw.loader.getState looks great and does exactly what you want. It seems like it would also be resilient to changes elsewhere (for example if one of those modules isn't loaded on certain pages this code also wouldn't load)

@Tacsipacsi mw.config variables are not currently covered by the frontend stable policy.

The problem is exactly that the policy doesn’t cover them at all – it states neither that they’re a stable interface nor that they aren’t. There are config variables (e.g. wgPageName) that are used all over the place and hopefully nobody would remove without deprecation, which makes one think that mw.config is in general a stable interface. This should be addressed in the policy, either by introducing a way to mark certain variables stable, or by introducing a totally new store for stable variables.

This is off topic but feel free to discuss on the stable policy talk page. If we were to make them stable we'd also need some practical way to deprecate them too which currently does not exist - feel free to create a Phabricator ticket for discussing that too.