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

Description

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

Steps to reproduce:

Go to test.wikipedia.org using a mobile device or mobile view on desktop
Login if you're not logged in already
Edit an article, start in VE mode
Toggle to source editing
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

Details

	Subject	Repo	Branch	Lines +/-
	Remove unnecessary addPlugin wrapper	mediawiki/extensions/WikimediaEvents	wmf/1.33.0-wmf.12	+6 -8
	Remove unnecessary addPlugin wrapper	mediawiki/extensions/WikimediaEvents	master	+6 -8

Customize query in gerrit

Related Objects

Mentioned In: T215367: [Regression wmf.16] Switching to VE from Wikitext editor is failing on mobile, shows error "TypeError: Cannot read property 'saveFields' of null"
T212974: Issues with switching between wikitext editor and VE
Mentioned Here: T212974: Issues with switching between wikitext editor and VE
T213883: Mobile: After switching from wikitext to visual and to wikitext again, page URL is not updated (causes the editor to not open again)
T209132: Add ability to set a revision tag by passing a URL parameter to the Visual Editor

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

kostajh added a subscriber: Etonkovidova.Jan 9 2019, 9:06 PM

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

Etonkovidova mentioned this in T212974: Issues with switching between wikitext editor and VE .Jan 10 2019, 12:42 AM

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

SBisson subscribed.Jan 10 2019, 2:36 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

gerritbot added a project: Patch-For-Review.Jan 10 2019, 4:09 PM

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

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

kostajh moved this task from Incoming to QA on the Growth-Team (Sprint 0 (Growth Team)) board.Jan 10 2019, 5:51 PM

ReleaseTaggerBot added a project: MW-1.33-notes (1.33.0-wmf.13; 2019-01-15).Jan 10 2019, 6:00 PM

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

ReleaseTaggerBot edited projects, added MW-1.33-notes (1.33.0-wmf.12; 2019-01-08); removed MW-1.33-notes (1.33.0-wmf.13; 2019-01-15).Jan 10 2019, 8:00 PM

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)

matmarex moved this task from Incoming to QA on the VisualEditor (Current work) board.Jan 10 2019, 8:44 PM

Changing to High as the backport was deployed.

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."

Edit an article, start in VE mode

Toggle to source editing

Toggle back to VE editing

the issue with problem switching and incorrect displayed editor options (previously partially reported in T212974: Issues with switching between wikitext editor and VE ) was filed as a new phab task - T213883

Liuxinyu970226 unsubscribed.Jan 16 2019, 1:32 AM

matmarex mentioned this in T215367: [Regression wmf.16] Switching to VE from Wikitext editor is failing on mobile, shows error "TypeError: Cannot read property 'saveFields' of null".Feb 6 2019, 12:51 PM

Toggling from VE to source to VE in MobileFrontend breaksClosed, ResolvedPublicActions

Description

Details

Related Objects

Event Timeline

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