• Bug 45021 has been marked as a duplicate of this bug. ***

Confirmed and hunt down.

From what I can see the problem may be in ve.dm.Surface not emitting any 'transact' events.

Adding a log() call in ve.init.mw.ViewPageTarget.prototype.onSurfaceModelTransact, shows me the following.

• When entering the first characters on a new page it is called. The callback then properly unbinds itself after the first call.
• Go to "Review and save" Now it binds the event again, so that it is notified of the first change (if ever) when the dialog is closed. This happens in ve.init.mw.ViewPageTarget.prototype.onShowChanges
• Close the dialog
• Altering the document again

the method is not called

Which I assume means ve.dm.Surface for some reason didn't emit any - or mw.ViewPageTarget is binding it wrong, but that looks fine to me.

@Roan, any ideas?

(In reply to comment #2)

Which I assume means ve.dm.Surface for some reason didn't emit any - or
mw.ViewPageTarget is binding it wrong, but that looks fine to me.

@Roan, any ideas?

I'm pretty sure ve.dm.Surface emits the events correctly. You can check this by placing a breakpoint on the line where Surface calls this.emit().

It's possible there's a bug in EventEmitter with detaching and reattaching though.

(In reply to comment #3)

(In reply to comment #2)

Which I assume means ve.dm.Surface for some reason didn't emit any - or
mw.ViewPageTarget is binding it wrong, but that looks fine to me.

@Roan, any ideas?

I'm pretty sure ve.dm.Surface emits the events correctly. You can check this
by
placing a breakpoint on the line where Surface calls this.emit().

It's possible there's a bug in EventEmitter with detaching and reattaching
though.

Hm.. could be. All I know is that the attached callback didn't get fired.

Using the following debug calls:

diff --git a/modules/ve/init/mw/targets/ve.init.mw.ViewPageTarget.js b/modules/ve/init/mw/targets/ve.init.mw.ViewPageTarget.js
index 859d4f8..cee2505 100644

• a/modules/ve/init/mw/targets/ve.init.mw.ViewPageTarget.js

+++ b/modules/ve/init/mw/targets/ve.init.mw.ViewPageTarget.js
@@ -390,6 +390,7 @@ ve.init.mw.ViewPageTarget.prototype.onSaveError = function ( jqXHR, status ) {

• @param {string} diffHtml */ ve.init.mw.ViewPageTarget.prototype.onShowChanges = function ( diffHtml ) {

+ mw.log( 'onShowChanges', 'Invalidate the diff (by calling proxiedOnSurfaceModelTransact) on transact event' );

// Invalidate the diff on next change
this.surface.getModel().on( 'transact', this.proxiedOnSurfaceModelTransact );

@@ -562,6 +563,7 @@ ve.init.mw.ViewPageTarget.prototype.onToolbarFeedbackToolClick = function () {

• @param {ve.dm.Transaction} tx Processed transaction */ ve.init.mw.ViewPageTarget.prototype.onSurfaceModelTransact = function () {

+ mw.log( 'onSurfaceModelTransact', 'Clear the diff html' );

// Clear the diff
this.$saveDialog
        .find( '.ve-init-mw-viewPageTarget-saveDialog-slide-review .ve-init-mw-viewPageTarget-saveDialog-viewer' )

@@ -691,6 +693,9 @@ ve.init.mw.ViewPageTarget.prototype.setUpSurface = function ( doc ) {

this.surface = new ve.Surface( this, doc, this.surfaceOptions );
this.surface.getContext().hide();
this.$document = this.surface.$.find( '.ve-ce-documentNode' );

+ this.surface.getModel().on( 'transact', function () {
+ mw.log( 've.Surface/Model/on/transact' );
+ } );

this.surface.getModel().on( 'transact', this.proxiedOnSurfaceModelTransact );
this.surface.getModel().on( 'history', this.proxiedOnSurfaceModelHistory );
// Transplant the toolbar

I've determined that the "transact" event does indeed continue to be emitted. But onSurfaceModelTransact only gets called once. It is not being rebound:

1. Page load, enter text ve.Surface/Model/on/transact load.php:11256
2. Press "Review and save" onSurfaceModelTransact Clear the diff html load.php:11256
3. Close dialog
4. Enter more text ve.Surface/Model/on/transact load.php:11256 ve.Surface/Model/on/transact load.php:11256
5. Press "Review and save"
6. Close dialog
7. Enter more lots more text (14x) ve.Surface/Model/on/transact load.php:11256

The comments above are wrong. Actual timeline:

Page load, enter text

ve.Surface/Model/on/transact
onSurfaceModelTransact Clear the diff html
ve.Surface/Model/on/transact

(2x) ve.Surface/Model/on/transact

1. Press "Review and save"
2. Close dialog
3. Enter more text

(3x) ve.Surface/Model/on/transact

1. Press "Review and save"
2. Close dialog

Related URL: https://gerrit.wikimedia.org/r/62847 (Gerrit Change I9eddebbdf9294ee3d46286bdf1b157e00252d300)

Code now merged; thanks, Timo! Will go out in next branch.