Inline Comments triggering "Confirm Reload" page on beforeunload

[boonebgorges]

FEE gives a saveAlert if you try to leave with a "dirty", unsaved post. Inline Comments changes post_content to add its markup, which causes FEE's isDirty() check always to be tripped once Inline Comments has done its thing - whether a real edit has taken place or not.

[christianwach]

I can't seem to replicate this either locally or on cdev. The 'change-tracking' branch has code to prevent this so if it's required on vanilla SP it can easily be ported.

[boonebgorges]

Weird. I'm now using a different machine, and I'm not seeing the problem. It must be a plugin conflict, or maybe a difference in load order caused by config difference between the two installations. (I'm guessing Inline Comments is loading really late or something.) I'll leave this ticket open, because it may be worth porting the copy_original() trick to somewhere later in the pageload at some point.

[christianwach]

I suspect load order too, but can't be sure without finding a way to reliably replicate.

[boonebgorges]

So, funny thing - on the installation where I'm having the problem, commenting out copy_original() actually solves it. Let me dig some more....

[r-a-y]

I was looking into an issue where FEE was causing unnecessary post revisions being created due to WP autosave and FEE's formatting filters and thought it was semi-related to this issue.

The commits that are linked to this ticket strip the extra paragraph elements caused by FEE applying wpautop() to the editor content.

This fixes the "There is an autosave of this post that is more recent than the version below" message from always appearing when toggling the "Enable Editing" button.

So, funny thing - on the installation where I'm having the problem, commenting out copy_original() actually solves it. Let me dig some more....

I also found this to be the case.

When I comment out the copy_original() line, this fixes the issue of the "The changes you made will be lost if you navigate away from this page." dialog appearing when I toggle the "Enable Editing" button on and off.

[boonebgorges]

@christianwach I'm currently getting the behavior on cdev. Any chance you could debug there, and/or have a think about what @r-a-y says above? I think we've got to fix this before we ship.

[christianwach]

@boonebgorges I cannot get any pages to load on cdev - all I get is ERR_CONNECTION_TIMED_OUT. I will look at this locally for the time being.

[christianwach]

On my local install, the listeners in 807f402 are attached prior to WP FEE's and are therefore able to manipulate the editor content before WP FEE checks it.

It'd be nice to strip the attributes from the post content before it's sent to the server during autosaves too. This would avoid the need to pre-filter the post content in e036fd2. However, I can't find an appropriate hook to do so - there's before-autosave but it's called after the content comparison is made. Any thoughts?

[r-a-y]

This would avoid the need to pre-filter the post content in e036fd2. However, I can't find an appropriate hook to do so - there's before-autosave but it's called after the content comparison is made. Any thoughts?

Yeah, I couldn't find an appropriate JS trigger, which made me filter the post content on auto-save.

On my local install, the listeners in 807f402 are attached prior to WP FEE's and are therefore able to manipulate the editor content before WP FEE checks it.

This fixes the "Enable Editing" toggling issue for me. Thanks Christian!

[boonebgorges]

Fixing it for me too. Thanks, @christianwach ! Let's reopen if it crops up again.

[christianwach]

As per Gitter comments, I'm reopening this because I have noticed that when the isDirty dialog appears, if I click "Cancel", then the paragraphs in the editor are left without the Inline Comments data-incom attributes - which of course breaks change tracking.

[christianwach]

This seems to be partially solved by 89c3d09 to the extent that the "Cancel" now reinstates the data-incom attributes in the editor content. I'm still unsure what conditions trigger isDirty as I explained on Gitter. To recap:

I've noticed that sometimes the editor considers a single delete/backspace key press not significant enough to trigger isDirty. This seems to depend on the length of the content. Try, for example, the following content in a saved paper:

One

Two

Three

Four

Five

This is equivalent to the following HTML once it's rendered:

<p>One</p>
<p>Two</p>
<p>Three</p>
<p>Four</p>
<p>Five</p>

Hit delete/backspace once with the cursor at the beginning of "Five" to produce "FourFive". For me, when I click "Disable Editing" I sometimes get the "Leave" dialog, sometimes not. Doesn't seem to be consistent, unfortunately.

I can force isDirty on the first content-changing keypress but I'm concerned that this could be fragile, given the variety of ways there are to change the content.

[christianwach]

206b11e resets comment data-incom-comment attributes when there are unsaved changes and "Leave" is clicked in the isDirty dialog.

[boonebgorges]

Wanted to make sure that the discussion from #77 (comment) was looped back here - it's possible that rolling back the copy_original() strategy, and coming up with an alternative way of determining dirtiness with respect to incom elements, may fix this problem as well as #77 and #71.

[christianwach]

@boonebgorges I'm working on a substitute for copy_original(). Should be able to report back later this evening.