Make WordPress Core

Opened 11 years ago

Closed 11 years ago

#28037 closed defect (bug) (fixed)

Editor merges paragraphs when post edit screen is accessed by pressing browser's back button

Reported by: dbernar1's profile dbernar1 Owned by: azaozz's profile azaozz
Milestone: 4.0 Priority: normal
Severity: normal Version: 4.0
Component: Editor Keywords: needs-patch
Focuses: javascript, administration Cc:

Description

Hello,

An editor at my company reported this problem. Here is one set of steps that can be used to reproduce the issue:


  1. Access the "Add New Post" screen
  2. Enter 2 words on separate lines into the Editor.
  3. Press "Save Draft"
  4. Press "View Post" in the admin toolbar
  5. Press your browser's back button

The two words will appear on the same line.


I've confirmed this issue can be reproduced with a brand new install of trunk, but I do not know how to troubleshoot this to solve it myself.

Thanks for your help and your work with WordPress!

Attachments (2)

28037.patch (829 bytes) - added by azaozz 11 years ago.
28037.2.patch (882 bytes) - added by azaozz 11 years ago.

Download all attachments as: .zip

Change History (23)

#1 @dbernar1
11 years ago

I should mention that I can reproduce this in current Chrome 34.0.1847.131

At the moment I can not reproduce it with Firefox 28.0

Last edited 11 years ago by dbernar1 (previous) (diff)

#2 @azaozz
11 years ago

  • Milestone changed from Awaiting Review to 4.0

Caused by Chrome's blatant disregard for autocomplete="off". Somewhat similar to #24364 but now happening for a textarea. Setting autocomplete="off" for the whole form works but affects all form fields, even fields added by plugins including postboxes.

Testing in Chrome 34, this happens even with the "Enable Auto-fill to fill in web forms in a single click." setting turned off...

@azaozz
11 years ago

#3 follow-up: @azaozz
11 years ago

Testing more: even setting the textarea to disabled or readonly doesn't stop Chrome from overwriting the content.

28037.patch resets the textarea value to the initial content by using textarea.textContent. Seems to work properly for Chrome for now but needs more testing, especially in other WebKit browsers.

Still, the best fix is to set autocomplete="off" on the form in Chrome. Can also be done with 'post_edit_form_tag' from a plugin.

#4 in reply to: ↑ 3 @dbernar1
11 years ago

Replying to azaozz:

Still, the best fix is to set autocomplete="off" on the form in Chrome. Can also be done with 'post_edit_form_tag' from a plugin.

Thank you very much. I've applied that for now.

#5 @azaozz
11 years ago

#28525 was marked as a duplicate.

#6 follow-up: @archon810
11 years ago

Could the solution involving setting autocomplete on the form be posted here please?

#7 in reply to: ↑ 6 ; follow-up: @dbernar1
11 years ago

Replying to archon810:

Could the solution involving setting autocomplete on the form be posted here please?

Hi Archon810,

I created a file named disable-autocomplete-for-editor.php in wp-content/mu-plugins, with the content shown in this gist: https://gist.github.com/dbernar1/2452771e7f935c89ebf8

#8 in reply to: ↑ 7 @archon810
11 years ago

Thank you, works great and is indeed very straightforward.

Replying to dbernar1:

Replying to archon810:

Could the solution involving setting autocomplete on the form be posted here please?

Hi Archon810,

I created a file named disable-autocomplete-for-editor.php in wp-content/mu-plugins, with the content shown in this gist: https://gist.github.com/dbernar1/2452771e7f935c89ebf8

#9 @iseulde
11 years ago

This is also a problem in Safari now... Really annoying.

#10 @iseulde
11 years ago

@azaozz, can't we work around this when TinyMCE loads? Maybe store the content somewhere else as well, and compare BeforeSetContent?

This ticket was mentioned in IRC in #wordpress-dev by DrewAPicture. View the logs.


11 years ago

#12 follow-up: @DrewAPicture
11 years ago

  • Owner set to azaozz
  • Status changed from new to reviewing

@azaozz: What are our options here to move a fix forward for Safari and Chrome?

#13 in reply to: ↑ 12 ; follow-up: @azaozz
11 years ago

Replying to DrewAPicture:

Re-testing the patch, it still seems to work in all the WebKit browsers I have access to (latest "desktop" Chrome and Safari, iOS7.1 Chrome and Safari, Android KitKat). However it is a "dangerous" thing to do.

There is no way to detect if the page is being loaded because the user clicked the Back or Forward button, so that code has to run on every TinyMCE init. The risk is that some old(er) or "obscure" versions of WebKit based browsers may not work properly when replacing textarea.value with textarea.textContent. That could make the editor completely unusable.

Replying to avryl:

This is possible in theory, however passing the content through wpautop() twice may break it in some cases. So we will have to determine if the content was passed through wpautop() first. As far as I see such test may not be 100% accurate.

IMHO the best change that will also fix this is to always load the post_content before is was filtered with wpautop(), i.e. ditch wp_richedit_pre() completely and only use wp_htmledit_pre(). Then add another 'BeforeSetContent' in the 'wordpress' plugin that will run the content through the JS wpautop() before it is loaded in MCE.

Been thinking about doing this (and testing it) few times in the past. It works well however may bring some back-compat issues. Can probably try it in 4.1?

For now I think the safest option is to set autocomplete="off" on the whole form in all WebKit browsers. This works well but would prevent autocomplete for all fields, including all metaboxes added by plugins. There will likely be some unhappy users because of this.

So the question is: what is more important, maintain the editor content when the user clicks the Back button or have autocomplete for metaboxes?

Last edited 11 years ago by azaozz (previous) (diff)

#14 in reply to: ↑ 13 ; follow-up: @archon810
11 years ago

Replying to azaozz:
I think the answer is obvious - one actively destroys content integrity, and the other disables autocomplete, a non-critical nuisance. Which one sounds more important? Because to me, the first one is infinitely more important than the latter.

#15 in reply to: ↑ 14 ; follow-up: @azaozz
11 years ago

Replying to archon810:

Right, if you put it this way the choice is obvious. However after clicking the Back button and seeing the "merged paragraphs" content, the users can reload the page to get it fixed. Suspecting this is what's happening now. So the choices are one inconvenience vs. another.

On the other hand I agree the user experience when clicking the Back button is higher priority than having autocomplete. We should add autocomplete="off" with a filter, so it is easily removable by people that hate it :)

Last edited 11 years ago by azaozz (previous) (diff)

#16 in reply to: ↑ 15 @archon810
11 years ago

Replying to azaozz:

Replying to archon810:

Right, if you put it this way the choice is obvious. However after clicking the Back button and seeing the "merged paragraphs" content, the users can reload the page to get it fixed. Suspecting this is what's happening now. So the choices are one inconvenience vs. another.

On the other hand I agree the user experience when clicking the Back button is higher priority than having autocomplete. We should add autocomplete="off" with a filter, so it is easily removable by people that hate it :)

You're assuming that a) people will notice and b) people will realize that it's the back button that messes things up and reloading fixes it.

Believe it or not, with an average post of 3 paragraphs, with some pictures sprinkled in, it's not even obvious unless you're paying close attention that some paragraphs got merged. Sometimes it's just 1 or 2 of them.

It took me several weeks to properly figure out the reason and find this thread (mostly because it was happening to my writers seemingly randomly and I blamed them at first), then figure out the right solution and spot the workaround. In the meantime, I had to instruct them to never hit the back button and reload. When you have a dozen writers with varying attention spans, snags like this one are much more of a pain in the ass than you might think. Definitely when compared to having no autocomplete.

#17 @dbernar1
11 years ago

In my experience the user also did not know what to do, and was actually manually re-inserting newlines when he would notice the issue.

Good idea to allow turning off the autocomplete="off" for those that run into problems with it.

This ticket was mentioned in IRC in #wordpress-dev by DrewAPicture. View the logs.


11 years ago

#19 @DrewAPicture
11 years ago

  • Keywords needs-patch added

Sounds like we need a new patch that disables autocomplete in the post editor. Any takers?

@azaozz
11 years ago

#20 @azaozz
11 years ago

28037.2.patch should do it. Uses the post_edit_form_tag action so plugins can remove it if needed.

#21 @azaozz
11 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 29448:

Editor: use the post_edit_form_tag action to add autocomplete="off" to the whole form on the Add/Edit Post screen in WebKit. Prevents editor problems when the browser's Back button is used. Fixes #28037.

Note: See TracTickets for help on using tickets.