Page MenuHomePhabricator

Toggling from VE to source to VE in MobileFrontend breaks
Closed, ResolvedPublic

Description

Error logged is: jQuery.Deferred exception: null is not an object (evaluating 've.init.target.saveFields')

Steps to reproduce:

  1. Go to test.wikipedia.org using a mobile device or mobile view on desktop
  2. Login if you're not logged in already
  3. Edit an article, start in VE mode
  4. Toggle to source editing
  5. Toggle back to VE editing

Toolbar disappears, and this exception is logged.

It's coming from this code:

mw.libs.ve.targetLoader.addPlugin( function () {
	mw.hook( 've.activationComplete' ).add( function () {
		if ( !ve.init.target.saveFields.campaign ) {
			ve.init.target.saveFields.campaign = function () {
				return mw.util.getParamValue( 'campaign' ) || '';
			};
		}
	} );
} );

in extensions/WikimediaEvents/modules/ve-wme/campaigns.js

Event Timeline

kostajh triaged this task as High priority.Jan 9 2019, 9:06 PM
kostajh created this task.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJan 9 2019, 9:06 PM

@matmarex could you take a look please, as this code was added as part of T209132 ?

So ve.init.target is null when this runs. This should not be possible. There might be some other underlying issue preventing the target from being set up? Or the hook is somehow broke on mobile. (Should also test this with switching between VE and NWE on desktop.)

I will investigate this tomorrow, if no one else does it earlier.

We can also probably just revert https://gerrit.wikimedia.org/r/475896 for now, I don't know when that is supposed to be used, but it doesn't seem to be in use yet.

kostajh updated the task description. (Show Details)Jan 10 2019, 2:12 PM
kostajh renamed this task from jQuery.Deferred exception: null is not an object (evaluating 've.init.target.saveFields') to Toggling from VE to source to VE in MobileFrontend breaks.Jan 10 2019, 2:20 PM
kostajh raised the priority of this task from High to Unbreak Now!.
kostajh added a project: MobileFrontend.
kostajh updated the task description. (Show Details)
Restricted Application added subscribers: Liuxinyu970226, TerraCodes. · View Herald TranscriptJan 10 2019, 2:20 PM

The problem is not the hook, but the unnecessary addPlugin call, which runs every time the target is created. The second time it runs, it fires the hook immediately to catch up with previous calls, but the new target hasn't been activated yet.

Change 483451 had a related patch set uploaded (by Esanders; owner: Esanders):
[mediawiki/extensions/WikimediaEvents@master] Remove unnecessary addPlugin wrapper

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

Change 483451 merged by jenkins-bot:
[mediawiki/extensions/WikimediaEvents@master] Remove unnecessary addPlugin wrapper

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

Buggy patch was included in 1.33.0-wmf.9 and 1.33.0-wmf.12. I'm going to backport to wmf.12 only, as wmf.9 is about to be undeployed later today, and the SWAT slot is already crowded (https://wikitech.wikimedia.org/wiki/Deployments#deploycal-item-20190110T1900).

Change 483485 had a related patch set uploaded (by Bartosz Dziewoński; owner: Esanders):
[mediawiki/extensions/WikimediaEvents@wmf/1.33.0-wmf.12] Remove unnecessary addPlugin wrapper

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

Change 483485 merged by jenkins-bot:
[mediawiki/extensions/WikimediaEvents@wmf/1.33.0-wmf.12] Remove unnecessary addPlugin wrapper

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

Mentioned in SAL (#wikimedia-operations) [2019-01-10T20:02:10Z] <tgr@deploy1001> Synchronized php-1.33.0-wmf.12/extensions/WikimediaEvents/modules/ve-wme/campaigns.js: SWAT: [[gerrit:483485|Remove unnecessary addPlugin wrapper (T213338)]] (duration: 00m 53s)

greg lowered the priority of this task from Unbreak Now! to High.Jan 11 2019, 11:31 PM
greg added a subscriber: greg.

Changing to High as the backport was deployed.

Etonkovidova closed this task as Resolved.EditedJan 16 2019, 12:57 AM
Etonkovidova claimed this task.

Checked in testwiki (wmf.13) - it seems that the fix Edit an article, start in VE mode

  • there is an additional step between (3) and (4) - "You must save your edit before switching to another editing mode."
  1. Edit an article, start in VE mode
  2. Toggle to source editing
  3. Toggle back to VE editing