Page MenuHomePhabricator

Edits with VeForAll in PageForms doesn't save to the page
Closed, ResolvedPublic

Description

Edits with VE in PageForms doesn't save to the page.

It saves just fine if you switch to source code mode.

The problem is in lack of textarea content update after VE chagnes (for example, you add something in VE, but textarea remains same).
Looks like pageforms use textarea as a source of it's content.

I resolved it temporarily changing resources/VEForAll.js :

	/**
	 * Being called by PageForms on textareas with 'visualeditor' class present
	 *
	 * @return {boolean}
	 */
	jQuery.fn.applyVisualEditor = function () {


		console.log(this);

		var config = mw.config.get( 'VEForAll' );
		if ( !config.VisualEditorEnable ) {
			return false;
		}

		return this.each( function () {

			var textarea = this,
			veEditor = new mw.veForAll.Editor( this, $( this ).val() );
			veEditor.initCallbacks.push( function () {
	
			});

			veEditor.initCallbacks.push( function () {
				veEditor.target.on( 'editor-ready', function () {

					//Catch any changes in Visual Editor.
					var MutationObserver    = window.MutationObserver || window.WebKitMutationObserver;
					var myObserver          = new MutationObserver (mutationHandler);
					var obsConfig           = { childList: true, characterData: true, attributes: true, subtree: true };
					var nodeToObserve = $( textarea ).parent().find('.ve-init-sa-target-surfaceWrapper').slice(-1)[0];
					myObserver.observe (nodeToObserve, obsConfig);

					function mutationHandler (mutationRecords) {
						veEditor.target.updateContent().then( function () {
							$( textarea ).trigger( 'change' );
							console.log('changed!');
						});
					}

					// Catch keyup events on raw textarea to use changes warning on page reload
					veEditor.target.$node.on( 'keyup', function () {
						$( textarea ).trigger( 'change' );
					} );
				} );
			} );


			// Add textarea updates for the case when textarea appared after page load. 
			// It is possible with multiple-instance templates in pageforms
			if ( typeof veEditor.target !== 'undefined' ){
				veEditor.target.on( 'editor-ready', function () {
					console.log('editor-ready');
                                        //Catch any changes in Visual Editor.
                                        var MutationObserver    = window.MutationObserver || window.WebKitMutationObserver;
                                        var myObserver          = new MutationObserver (mutationHandler);
                                        var obsConfig           = { childList: true, characterData: true, attributes: true, subtree: true };
                                        var nodeToObserve = $( textarea ).parent().find('.ve-init-sa-target-surfaceWrapper').slice(-1)[0];
                                        myObserver.observe (nodeToObserve, obsConfig);

                                        function mutationHandler (mutationRecords) {
                                                veEditor.target.updateContent().then( function () {
                                                        $( textarea ).trigger( 'change' );
                                                        console.log('changed!');
                                                });
                                        }

                                        // Catch keyup events on raw textarea to use changes warning on page reload
                                        veEditor.target.$node.on( 'keyup', function () {
                                                $( textarea ).trigger( 'change' );
                                        } );

				} );
			}

			veInstances.push( veEditor );
		} );
	};

There are a lot of problems with such solution but the main are:

  1. Probably using MutationObserver is a bad idea. But I don't know how to subscribe to VE content changes. Using
veEditor.target.getSurface().getView().on('keyup'

is a bad idea because that event does not cover if user just clicked on a toolbar button (to make text bold, for example).

  1. I've added the same code twice to resolve multiple-instances templates. Probably there is more suitable callback instead of that solution.

Event Timeline

Urfiner created this task.Mar 29 2019, 3:32 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 29 2019, 3:32 PM
Urfiner updated the task description. (Show Details)Mar 29 2019, 3:39 PM
tosfos added a subscriber: tosfos.Apr 29 2019, 2:22 PM

Temporary hack around it - interval

var watchChanges = setInterval(function(){
var allEditors = $.fn.getVEInstances();
for(var editor of allEditors){
if(editor.target.getSurface().$element.find('.ve-ce-surface-focused').length){
var html = editor.target.getSurface().getHtml();
editor.target.$element.closest('.ve-area-wrapper').find('textarea').val(html);
}
}
},1000);

You can put it in external file.

Vedmaka claimed this task.Jul 29 2019, 7:52 PM
Anysite added a comment.EditedJul 29 2019, 8:31 PM

Vedmaka - new approach is await in gerrit.

In addition - add this to resources/VEForAll.js

$('body').on('focus','.ve-ce-surface', function(){
     for( let instance of veInstances){
         //find instance that own this sourface
         if( this == instance.target.getSurface().getView().$element.get(0)){
            //When editor loses focus, update the field input.
            instance.target.focusedWithoutUpdate = true;
      }
   }
});

@Urfiner can I ask you to verify your PageForms version and commit one more time?
@Anysite could you please share your PageForms and VEForAll version too?

Any chance you have some extra PageForms plugins installed?

The reason I am asking is because not updating connected textarea except when editor mode is being switched - is a legit behaviour of VEForAll. Instead the sync event is being initiated from PageForms upon form submission (and preview/save and continue) actions [1].

The weird part is that PageForms and VEForAll versions you have specified in bug report seems to be ok and already include mentioned changes, so that's why I am asking about extra plugins installed.

  1. https://phabricator.wikimedia.org/rEPFMda0922d818ab030df0c8b1de32b52cfc6166fcce

But sync event is not enough, because VEForAlll updated the textareas asynchronously.

So

  • Pageform fired sync event
  • VEForAlll started to update the textareas asynchronously.
  • The post action of the form could be fired before textarae update finishes.
  • And sometimes it could all be O.K., and you would not recognize the bug, because that's the nature of async.

Maybe you could test it by make the parsoid response very slowly.

@Anysite that's true, but the point is that PageForms code actually do wait for VEForAll to complete all the work and only then actually submits the form [1]

  1. See libs/PF_submit.js at https://phabricator.wikimedia.org/rEPFMda0922d818ab030df0c8b1de32b52cfc6166fcce line 221 and lines 177 - 189

I see now.
Yes, we still using PageForms 4.4.1.
We would check the new version, Thank you.

Vedmaka closed this task as Resolved.Aug 1 2019, 9:29 AM

Oh, I see, I am glad we've found it! @Urfiner could you please verify your PF version too?