WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 7 years ago

Last modified 6 years ago

#23042 closed defect (bug) (fixed)

Post edit page may become almost unusable and textarea resize not saved

Reported by: kinderr@… Owned by: azaozz
Milestone: 3.5.1 Priority: normal
Severity: normal Version: 3.5
Component: Editor Keywords: has-patch
Focuses: Cc:
PR Number:

Description (last modified by SergeyBiryukov)

Follow-up testing to Ticket #22708 and patches [23016]
[23017]

I installed the new versions of:
/wp-admin/js/post.js (23016) and
/wp-includes/class-wp-editor.php (23017)

on my Wordpress 3.5 server. The problem still exists that the post editor window is forced to 5000 pixels tall because the wp_usermeta -> wp_user-settings -> ed_size value is being set to ed_size=310566666&editor=tinymce&mfold=f or other extremely large value with FireFox 17.0.1 and Chrome 23.0.1271.97 m. The ed_size bad cookie value triggers the patch code logic to set the post editor to 5000 lines.

My Win7 screen resolution is 1920w x 1080h which means the editor is around 5 screens tall. This is almost as bad as the practically infinite bug value. Manually resizing the post editor height by dragging the lower right pane control will not survive a post update and the post editor resets to 5000 pixels high. This makes the editor tool bar practically inaccessible and a big drag on usability. See http://wordpress.org/support/topic/the-option-to-change-the-size-of-editor-is-missing-how-to-change-the-size

As a temporary workaround, I modified line 704 in post.js to 500 pixels:

if ( height > 50 && height < 500 && height != getUserSetting( 'ed_size' ) )
setUserSetting( 'ed_size', height );
}

and lines 74 & 75 in class-wp-editor.php to 500 pixels:

if ( $set['editor_height'] < 50 )
  $set['editor_height'] = 50;
elseif ( $set['editor_height'] > 500 )
  $set['editor_height'] = 500;

which is a comfortable post editor size (my personal preference) that is "sticky".

In summary, the 5000 pixel sanity logic for the post editor size doesn't fix the underlying ed_size blowup.

Attachments (5)

23042.patch (4.2 KB) - added by azaozz 7 years ago.
post.js (23.3 KB) - added by SergeyBiryukov 7 years ago.
Patched file
23042-2.patch (4.9 KB) - added by azaozz 7 years ago.
23042-3.patch (4.0 KB) - added by azaozz 7 years ago.
23042-4.patch (3.9 KB) - added by azaozz 7 years ago.

Download all attachments as: .zip

Change History (28)

#1 @SergeyBiryukov
7 years ago

  • Description modified (diff)

#2 @knutsp
7 years ago

  • Cc knut@… added

#3 @nacin
7 years ago

  • Milestone changed from Awaiting Review to 3.5.1

#4 follow-up: @SergeyBiryukov
7 years ago

Sounds like a valid bug. Not sure how to reproduce it though, and where the huge ed_size value comes from. Is it returned by $('#wp-content-editor-container').height() in some cases?

#6 @kinderr@…
7 years ago

If you'd like to provide code with echo statements or dumping info. to a file, I'd be happy to run it in my staging environment.

#7 in reply to: ↑ 4 ; follow-up: @kymar
7 years ago

Replying to SergeyBiryukov:

Sounds like a valid bug. Not sure how to reproduce it though, and where the huge ed_size value comes from. Is it returned by $('#wp-content-editor-container').height() in some cases?

To cause the editor box to re-size at 5000px height on all subsequent saves and opens, you need only to save a post once at > 5000px height.

#8 @azaozz
7 years ago

In #22708 the code was changed to save only on resizing the editor (either Visual or Text). As far as I see the only explanation is that $('#wp-content-editor-container').height() returns a huge value in some rare cases... Not able to reproduce it either but will keep trying. Might be caused by getting the height and at the same time resizing a child element (not confirmed).

In the meantime perhaps can modify that core to look only at elements that have css height set, i.e. the iframe and the textarea. That will make it a bit of a guess when matching the height of #wp-content-editor-container when switching Visual->Text and back, but should return the actual pixel value.

#9 in reply to: ↑ 7 @azaozz
7 years ago

Replying to kymar:

To cause the editor box to re-size at 5000px height on all subsequent saves and opens, you need only to save a post once at > 5000px height.

The thing is that the JS won't save 'ed_size' if the height is > 5000: http://core.trac.wordpress.org/browser/trunk/wp-admin/js/post.js#L704 and http://core.trac.wordpress.org/browser/trunk/wp-admin/js/post.js#L759

The only logical explanation of how a height > 5000 can get in there is if the user has tested 3.5-beta when this was still broken. However the height should have been reset as soon as the user resized the editor.

@azaozz
7 years ago

#10 @azaozz
7 years ago

In 23042.patch​:

  • Converted the JS to use only elements that have css height set.
  • Updated to use jQuery .on() and .off() for events.

@kymar could you test if this patch fixes your problems. To test, apply the patch and set SCRIPT_DEBUG (so the non-minified files are loaded).

#11 @yoavf
7 years ago

  • Cc yoavf added

#12 follow-up: @kinderr@…
7 years ago

Hi,
Is there a ready-to-go downloadable post.js file? The 23042.patch link above is a co-mingled listing of deleted (red) and added (green) lines. Or just e-mail the new file to me.

Thanks,
Robert

@SergeyBiryukov
7 years ago

Patched file

#13 in reply to: ↑ 12 @SergeyBiryukov
7 years ago

Replying to kinderr@…:

Is there a ready-to-go downloadable post.js file?

Try http://core.trac.wordpress.org/raw-attachment/ticket/23042/post.js.

Don't forget to set SCRIPT_DEBUG in wp-config.php:
http://codex.wordpress.org/Debugging_in_WordPress#SCRIPT_DEBUG

#14 @kinderr@…
7 years ago

The new post.js works great! I can resize the editor and it's sticky. The editor pane vertical size is persistent across post edit sessions for both new and existing posts. No need to save the post, the editor size is kept.
--
[21:24:06.083] getUserSetting('ed_size');
[21:24:06.084] "504"
--
[21:27:40.104] getUserSetting('ed_size');
[21:27:40.105] "577"
--
[21:28:06.455] getUserSetting('ed_size');
[21:28:06.457] "196"

Staging System:

Wordpress 3.5 with the original class-wp-editor.php and updated post.js per this thread. i.e. Only post.js changed from the base Wordpress 3.5 install.

Windows 7 - 32 bit

FireFox 17.0.1

WampServer 2.2

Apache 2.2.22

PHP 5.4.3

MySQL 5.5.24

Also tested fine with Chrome v23.0.1271.97 m

Thanks,
Robert

#15 @nacin
7 years ago

  • Keywords has-patch added; needs-patch removed

#16 follow-up: @nacin
7 years ago

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

I am a bit concerned about the amount of churn in 23042.patch for 3.5.1. Anything we can do to make this more bit-sized?

@azaozz
7 years ago

#17 in reply to: ↑ 16 @azaozz
7 years ago

Replying to nacin:

Most of it comes from using the newer jQuery syntax for attaching events, .on() and .off() instead of .mousemove() and .mouseup() as it allows for simpler detaching.

The changes in 23042-2.patch are that it will reset the height if it's outside the normal range (50px -- 5000px). It also doesn't use jQuery(element).css('height') as this would return a calculated height instead of an empty string if the height is not set.

@azaozz
7 years ago

#18 @azaozz
7 years ago

After chatting in IRC last night, reverted the textarea resizing part keeping only the reuse of the height value after resizing. The part that changes it to use .on() and .off() can be added for 3.6 only.

The second part is more robust/includes more safeguards to avoid any possibility of getting a wrong height value. This is primarily by looking at the style="height: ..." for the elements eliminating any "calculated" values. jQuery(element).css('height') returns the browser calculated height not the value from the element's "style" attribute. That may explain the rare edge cases where height is around 17,000,000px.

It will also reset a previously saved huge value (more than 5000px) to the default height of 360px.

#19 @azaozz
7 years ago

There are still more details about this happening (or has happened after upgrading to 3.5), unfortunately nothing that can be reproduced consistently.

In that terms best would be to reset a huge ed_size in the cookie to the default of 360px. Then the worst case scenario will be the user loosing the custom height that may have been set, not ending up with a huge editor that is quite hard to manage.

@azaozz
7 years ago

#20 @azaozz
7 years ago

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

In 23302:

Main editor: when setting or saving the height, look only at elements that have style="height:..." set. Reset a previously saved erroneous "ed_size" value (over 5000px) to the default height of 360px. Fixes #23042 for trunk.

#21 @azaozz
7 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopen for 3.5.1.

#22 follow-up: @azaozz
7 years ago

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

In 23326:

Main editor: when setting or saving the height, look only at elements that have style="height:..." set. Reset a previously saved erroneous "ed_size" value (over 5000px) to the default height of 360px. Fixes #23042 for 3.5.

#23 in reply to: ↑ 22 @theatereleven
6 years ago

I'm seeing this problem now in WP 3.9. Anyone else?

Note: See TracTickets for help on using tickets.