isDirty should be cleared when settings are saved

[boonebgorges]

From https://commons.gc.cuny.edu/wp-login.php?action=bpnoaccess&auth=1&redirect_to=http%3A%2F%2Fcommons.gc.cuny.edu%2F%3Fp%3D44222

I opened one of my published papers.
I clicked Enable Editing
I made modifications to the settings panel (changed abstract, changed paper from public to private, added a reader)
I clicked Update and got the green pop up that the update took place.
I then attempted to navigate away from the page and got the prompt "The changes you made will be lost if you navigate away from the page. "Cancel" "Update and leave" "Leave"

I have experienced something along these lines in the past. It's possible that it's resolved with recent changes to isdirty logic.

[christianwach]

Perhaps this was tested before the inclusion of changes.js? This line forces isNotDirty after saving.

[boonebgorges]

Also

— If I save a draft and try to leave the page the the prompt “The changes you made will be lost if you navigate away from the page. “Cancel” “Update and leave” “Leave”. Same as what Stephen reported recently, but on a draft rather than published paper.

[boonebgorges]

Probably the same issue, from http://commons.gc.cuny.edu/?p=44277:

When I go in to approve comments on a paper and click ‘Approve’ ‘Spam’ or ‘Trash’, they all bring up the “The changes you made will be lost if you navigate away from this page. Cancel/Update & Leave/Leave” dialog. I would assume that I would be staying in the editing interface after choosing any of the moderation options and wouldn’t need to choose to update or save my changes or leave.

[boonebgorges]

A clue: During a recent edit, I got a note about a more recent autosave, and when I clicked through to look at it, I saw that the difference between the autosave and the latest revision was coming from wptexturize(). Maybe something similar is happening with the isDirty() check in FEE?

Perhaps this was tested before the inclusion of changes.js? This line forces isNotDirty after saving.

@christianwach I don't 100% understand how FEE intends to be handling isDirty(). It has its own isDirty() function, which checks to see whether the post_title and post_status match what's in postOnServer. This is what it checks on beforeunload. But in your changes code, you set isNotDirty on the TinyMCE editor, which FEE doesn't look at for the beforeunload notice. I wonder if this inconsistency is part of the issue. What do you think of unbinding FEEsbeforeonload.feecallback, and providing our own? That way, we could be more consistent about checkingeditor`, and also add in some flags related to Settings.

[r-a-y]

A clue: During a recent edit, I got a note about a more recent autosave, and when I clicked through to look at it, I saw that the difference between the autosave and the latest revision was coming from wptexturize(). Maybe something similar is happening with the isDirty() check in FEE?

It's a FEE problem. See my comment here:
#42 (comment)

I fixed the wpautop() autosave issue by reversing wpautop at the server level (commit e036fd2). I didn't do this for wptexturize() or convert_chars(). FEE relies on these filters otherwise the content looks messed up in the editor. However, these filters mess up autosave and revisions...

[boonebgorges]

8cdeb7f takes @christianwach's fix from 807f402 and replicates it for the comment moderation links, which seem to be the place where the team was noticing the issue the most.

There's still a ton of inconsistency here, and it would be nice to come up with a better system. Instead of doing content comparison, as FEE currently does, it would be less problematic to have a simple flag that gets tripped by element changes:

SocialPaper.isDirty = false;
$( 'input,textarea' ).on( 'change', function() { SocialPaper.isDirty = true; } );

// and whatever other clicks FEE currently checks
$window.on( 'beforeunload', function() {
    if ( SocialPaper.isDirty ) {
         whatever
    }
}

// and set SocialPaper.isDirty to false on a successful save

This would generate more false positives (when A changes to B and then back to A), and it would be subject to certain race conditions. But it would be much less finicky than what's currently in FEE.

That said, I don't know how to do it without hacking FEE. Ideally, isDirty() would be a method on wp.fee that could be easily overriddden. And in fact it is, except that FEE itself doesn't call wp.fee.isDirty(), it calls the locally-scoped isDirty(), which means that overriding the method has no effect.