Page MenuHomePhabricator

Edit link is missing from structured caption panel on Beta
Closed, ResolvedPublicBUG REPORT

Description

Structured captions can't be edited on Beta because the link to do so is missing.

Steps to Reproduce: Visit any file page on Commons
Actual Results: Edit link is missing
Expected Results: Edit link is there

The problem seems to be related to this change: https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Wikibase/+/523737/ in conjunction with a recent change in WikibaseMediaInfo to depend on mw.hook( 'wikibase.entityPage.entityLoaded' ).

Event Timeline

Triaging as high for now, but this maybe should be UBN and/or a train blocker.

Mholloway raised the priority of this task from High to Unbreak Now!.Jul 29 2019, 4:12 PM

Triaging as high for now, but this maybe should be UBN and/or a train blocker.

Can you give me a link?

Technically, it should not cause any issues on commons since commons is also a repo so it loads all of the moved modules regardless. Maybe there's an issue with caching

mw.hook( 'wikibase.entityPage.entityLoaded' ) isn't firing, afaics that's what's wrong

It's definitely not caused by 87ca76d5ee70fd5bdd0d09f8067182886bf867c5 so reverting that patch seems wrong.

You can see modules of WikibaseView all are loaded in commons:

mw.loader.getModuleNames().includes('dataValues.DataValue');
true

The likely cause is https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Wikibase/+/523737/ but that thing is already live

yep that looks like the likely cause ... it didn't cause a problem straight away because we weren't depending on the hook until a v recent change

yep that looks like the likely cause ... it didn't cause a problem straight away because we weren't depending on the hook until a v recent change

Do you load/call wikibase.ui.entityViewInit? If not, just add wikibase.entityPage.entityLoaded as dependency of your bootstrap js

Change 526181 had a related patch set uploaded (by Ladsgroup; owner: Ladsgroup):
[mediawiki/extensions/WikibaseMediaInfo@master] Add wikibase.entityPage.entityLoaded as dependency of the wikibase.mediainfo.filePageDisplay

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

When reproducing locally, this error output appears in the console: https://phabricator.wikimedia.org/P8818

those errors might be stemming from the fact the local instance does not have Wikibase git submodule updated (similar error seen locally here at Wikidata dev team). It would also explain why the error does not show up on beta, where submodules are up-to-date. Could you please double check this?

Meanwhile, we're investigating some other path.

@WMDE-leszek Yes, I think that's right. I'm not seeing it since updating my submodules. Will update the description to remove that error.

Change 526188 had a related patch set uploaded (by Ladsgroup; owner: Ladsgroup):
[mediawiki/extensions/Wikibase@master] Add mw.config.values.wbRepo as dependency of wikibase.entityPage.entityLoaded

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

Change 526189 had a related patch set uploaded (by Ladsgroup; owner: Ladsgroup):
[mediawiki/extensions/Wikibase@master] Fix lib js path

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

alaa_wmde lowered the priority of this task from Unbreak Now! to High.Jul 29 2019, 5:47 PM
alaa_wmde subscribed.

Code has not reached production. High is the hightest here (as a production error should be addressed before those that hasn't reached production yet)

So the issue seems a little bit more complicated than it looks. In short I recommend stop depending on the hook until we build a proper fix for federated systems for it or we can find a fix for it together.

The url that is being constructed uses mw.config.values.wbRepo. That value points to Wikidata but the special page needs to be invoked inside commons. If you don't use this value in your frontend, just changing it (through mw.config.set('wbRepo')) would fix the issue (+ plus merging all of patches mentioned above)

Change 526195 had a related patch set uploaded (by Ladsgroup; owner: Ladsgroup):
[mediawiki/extensions/Wikibase@master] Use wgArticlePath instead of wbRepo for wikibase.entityPage.entityLoaded

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

So the issue seems a little bit more complicated than it looks. In short I recommend stop depending on the hook until we build a proper fix for federated systems for it or we can find a fix for it together.

The url that is being constructed uses mw.config.values.wbRepo. That value points to Wikidata but the special page needs to be invoked inside commons. If you don't use this value in your frontend, just changing it (through mw.config.set('wbRepo')) would fix the issue (+ plus merging all of patches mentioned above)

I take this back, https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Wikibase/+/526195 would solve the issue. Thanks @Lucas_Werkmeister_WMDE for pointing out the potential fix.

Change 526188 abandoned by Ladsgroup:
Add mw.config.values.wbRepo as dependency of wikibase.entityPage.entityLoaded

Reason:
Done in I9a80f66206a439d8a74a53c10f8d73eb0dac2b0a

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

Change 526181 abandoned by Ladsgroup:
Add wikibase.entityPage.entityLoaded as dependency of the wikibase.mediainfo.filePageDisplay

Reason:
Not needed

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

@Cparle and @Mholloway Can you take a look at https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Wikibase/+/526195 ? This might fix the issue. Please test it locally and let me know.

I think there are actually a couple of different things going on here, both fundamentally resulting from our reliance on wikibase.entityPage.entityLoaded firing:

(1) The edit link never displays for items without any structured data, because the hook doesn't fire;
(2) Separately, on Beta Commons, the hook also doesn't fire because it's incorrectly attempting to get mediainfo from Wikidata Beta rather than Commons beta, and getting a 400 response.

@Ladsgroup I think your patch will address (2), correct? As for (1), it seems like we're mistakenly assuming that the hook will fire even if no data is found. But I wonder if it should fire even if no data is found, so long as there's no error?

Change 526195 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Use wgArticlePath instead of wbRepo for wikibase.entityPage.entityLoaded

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

Files on the Beta Cluster with existing mediainfo content do indeed have their caption panel edit links restored as a result of the patch from @Ladsgroup. Files with no mediainfo yet existing still have the edit link missing. @egardner is writing up a separate ticket to discuss exactly what to do about that.

Here is a new ticket for the second part of this bug: https://phabricator.wikimedia.org/T229279 – would be good to discuss how to proceed there.

Change 526181 restored by Ladsgroup:
Add wikibase.entityPage.entityLoaded as dependency of the wikibase.mediainfo.filePageDisplay

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

You can see modules of WikibaseView all are loaded in commons:

mw.loader.getModuleNames().includes('dataValues.DataValue');
true

FWIW that only tells that they are defined (ie. present in extension.json or some such), not that they are loaded. That would be mw.loader.getState('dataValues.DataValue') === 'ready' (which is also true).

brennen raised the priority of this task from High to Unbreak Now!.Jul 29 2019, 11:48 PM

Moving to UBN!, as this is a train blocker.

You can see modules of WikibaseView all are loaded in commons:

mw.loader.getModuleNames().includes('dataValues.DataValue');
true

FWIW that only tells that they are defined (ie. present in extension.json or some such), not that they are loaded. That would be mw.loader.getState('dataValues.DataValue') === 'ready' (which is also true).

I meant WikibaseView is loaded on Commons in beta cluster which registers the modules. (The patch mentioned in the task summary, stops loading WikibaseView on clients and was thought to be the reason behind this issue which seemed to be incorrect)

Change 526189 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Fix lib js path

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

It's back now, so it's not a train blocker anymore. Right?

Mholloway lowered the priority of this task from Unbreak Now! to High.Jul 30 2019, 3:59 PM

This is resolved and no longer blocking the train. Leaving open for QA.

Thanks everyone for working hard and fast on this to unblock the train! Amazing work

Ramsey-WMF subscribed.

Testing on production and this looks to be working fine. Thanks for the great, fast work everyone!