Page MenuHomePhabricator

Inspector does not open after reloading the page on desktop
Open, Needs TriagePublicBUG REPORT

Description

Steps to Reproduce:

  1. Navigate to an article with link recommendations from Special:Homepage
  2. Confirm the link inspector is shown
  3. Reload the page

Actual Results:

  1. The link inspector does not open

Expected Results:

  1. The link inspector opens

On mobile, this works fine, it's an issue with the desktop implementation

Event Timeline

I'm looking into this today. So far I've found that none of the code in AddLinkArticleTarget.js runs on page reload.

I'm looking into this today. So far I've found that none of the code in AddLinkArticleTarget.js runs on page reload.

FWIW the inspector loads if you switch from mobile to desktop, so that probably can give some clue of how to fix on desktop.

kostajh renamed this task from Inspector does not open after reloading the page to Inspector does not open after reloading the page on desktop.Tue, Mar 30, 8:10 AM

@Tgr mentioned in T277228: VE plugin fails to load on desktop article target that the current solution for loading add link plugin on desktop won't address the issue when the page is reloaded, so this issue might be fixed when the real fix for T277228 is in. I think we should keep this bug report separate for now though in case there are further issues that arise even when the plugin is loaded given that there are already a few issues w/link inspector upon initial load.

If I'm on the read page, and refresh, then click edit, I get AI suggestions. But it only doesn't work if I refresh on edit. Is there a way to use this? (Like @mewoph's earlier solution for T277228).

addPlugin (from mw.hook( 've.loadModules' )) doesn't do anything if loadModules in ve.init.mw.ArticleTargetLoader has already been called. In this case, ext.growthExperiments.SuggestedEdits.Guidance.js needs to be loaded and addPlugin needs to be called before loadModules is called.

I added some more logs and noticed that the sequences of events are different between the first load and the reload attempts.

First load — inspector shows up

Reload on edit — inspector doesn't show up

Reload on read & manually click edit — inspector shows up

The discrepancy in the sequence of events lines up with the different behaviors we are seeing here.

Given these constraints, for reload to work, I think we would either have to guarantee that guidance.js is loaded before VE or to not use addPlugin if loadModules has already been called by the time guidance.js loads. I'm not sure how to achieve the former. As for the latter, I tried checking for the presence of ve.init.platform and inspector showed up on reload.

In guidance.js:

			mw.hook( 've.loadModules' ).add( function ( addPlugin ) {
				// Either the desktop or the mobile module will be registered, but not both.
				// Start with both, filter out the unregistered one, and add the remaining one
				// as a VE plugin.
				[
					'ext.growthExperiments.AddLink.desktop',
					'ext.growthExperiments.AddLink.mobile'
				].filter( mw.loader.getState ).forEach( function ( module ) {
					if ( window.ve && ve.init && ve.init.platform ) {
						mw.loader.using( module );
					} else {
						addPlugin( module );
					}
				} );
			} );

@mepps @Tgr @kostajh — do you think this makes sense as a solution to this?

Is there a reason we need to use addPlugin if it works to load the modules without it?

Change 678012 had a related patch set uploaded (by Mepps; author: Mepps):

[mediawiki/extensions/GrowthExperiments@master] Use mw.loader.using instead of addPlugin to load AddLink

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

@mewoph I put up a patch that does not include the if statement because I found that worked for me locally. However, I understand there are nuances to the loading times, so if addPlugin is needed, feel free to update the patch I put up.

Thanks @mepps I'm not sure why addPlugin was used. Maybe @kostajh or @Tgr have more context?