onbeforeunload alert always triggers with attachments

[duck_]

Steps

1. Create a post with attachments in it
2. Visit the post's edit screen
3. Navigate away without modification and see an alert about "The changes you made will be lost..."

Trying to press "Update" and then leaving gives the same result.

Cause

tinymce.activeEditor.isDirty() is always returning true because attachments are lazy loaded with individual XHRs and so the editor's startContent property doesn't contain the full HTML for an attachment. Once an attachment model has been loaded then it's re-rendered and the editor content is different even though the user hasn't made any modifications.

Related: #21390, #21813, [22012]

A couple of options:

1. Some kind of bootstrapping for images instead of loading them in a separate XHR
2. Overwriting the editor's startContent

Option 2. has the potential issue of a slow loading image updating the startContent after a user has made some actual modifications and stopping the navigation warning from appearing.

Related/duplicate: #22048

#22048 was marked as a duplicate.

Discussed this with @koopersmith in IRC. Only happens when the user opens an existing post for editing but doesn't edit anything, caused by the replacement of the shortcodes with "views" when the editor loads the initial content.

There is a way to tell TinyMCE the content wasn't modified which will cancel the beforeunload event. We need a bit more logic to make sure the user hasn't started typing before all shortcodes were replaced with views. Patch coming soon.

Replying to azaozz:

There is a way to tell TinyMCE the content wasn't modified which will cancel the beforeunload event. We need a bit more logic to make sure the user hasn't started typing before all shortcodes were replaced with views. Patch coming soon.

We're not going to block the user, right? But rather, once all views are loaded, we should only set dirty to false if the user hasn't started typing by then.

Replying to nacin:

Right. Tested attaching an one-time-only callback to the keydown and paste events so we know when the user has started typing. If that occurs before all views are loaded, we leave the editor state as "dirty" (i.e. onbeforeunload will trigger). If the user hasn't started typing when all views are loaded, we set the editor state to "clean".

The other thing to think about is what happens when they try to leave a post before all of the views are loaded. If the one-time-only callbacks have yet to fire, we'll want to consider it to be clean.

Yeah, the logic is pretty simple: there's a var isClean = true; that is changed to false onpaste and onkeydown. Both callbacks are removed if any of them runs or when all views are loaded. After all views are loaded we check that var and set ed.isNotDirty = true if isClean is true.

It's a bit tricky to sync this with ed.isNotDirty while the views are loading, trying to figure out the best way (the onbeforeunload event in the browsers is a "special" one and doesn't behave as a standard event).

Removing views, see #21813 (comment 30), should resolve this for 3.5.

In 22567:

Media: Restore 3.4 editor behavior and remove TinyMCE views.

• Reactivates the wpgallery and wpeditimage TinyMCE plugins. Deactivates the wpviews TinyMCE plugin.
• Moves still-relevant logic from mce-views.js to media-upload.js and shortcode.js.
• No longer include wp-includes/js/mce-views.js. This code will not be used in 3.5, and should be considered unstable.
• Currently, this is the real 3.4 experience; as such, editing triggers the old modals. Changing this is the next major step.

When reassessing views, we should look over all of these tickets and anticipate these bugs accordingly.

fixes #21813, #22123, #22155, #22161, #22257, #22266, #22318, #22407, see #21390.

Another method we could use here is to avoid editor.isDirty() entirely, and compare processed versions of the content. Attaching a patch as an example.