Comment Quick Edit quirks

[garyc40]

Two problems exist from 3.0.3:

1. Wrong textarea height when editing a long comment (see screenshot)

2. Quicktags are not properly closed when canceling a quick edit form, or navigating to another page

there's a patch for that

Point 1 can be fixed with #18348.

Related: #16695

Refreshed the patch for point 2.

Since replycontent becomes empty after cancelling Quick Edit form, closeAllTags() should just clear button states (unlike CloseButton(), which actually changes content).

15911.3.patch​ gives the function a more appropriate title.

15911.4.patch​ fixes and keeps the old function as well. Currently it's not used anywhere though, so I prefer 15911.3.patch​.

Not 100% sure we need this but it is nice to stop "polluting" the textarea when it's empty and we still have open tags. Think we can add this directly in qt.TagButton.prototype.callback(), will make a patch soon.

In [18576]:

Fix QTags.closeAllTags(), replace 'tb' with 'ed' in quicktags,js to make it clear it is the editor instance not the toolbar, small comments quick edit fixes, see #16695, fixes #15911

When cancelling Quick Edit form with some open tags in it, calling closeAllTags() from commentReply.close() floods Firefox JS console (6.0/Windows, tried without extensions too) with errors:

Error: uncaught exception: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIDOMNSHTMLTextAreaElement.selectionStart]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: http://trunk.wordpress/wp-includes/js/quicktags.dev.js?ver=20110820 :: <TOP_LEVEL> :: line 410"  data: no]

And the comment disappears until page reload.

If it's called earlier (e.g. before fadeOut() in commentReply.revert()), everything works as expected, though we can briefly see the tags closing, and these changes will be discarded anyway.

That's why I thought about just clearing button states, without touching replycontent if it's invisible.

And the comment disappears until page reload.

Same in IE 8.

15911.5.patch​ introduces clearButtonStates(), which fixes the issue. If it should be fixed in some other way, I'd like to know.

Removing garyc as owner so a committer will look at ticket (when it is already "owned" by someone it gets ignored by the others).

In [19290]:

Prevent errors in QTags.closeAllTags(), make it remove button states but not modify the content (as that often is wrong anyways), fixes #15911

Replying to SergeyBiryukov:

Imho we shouldn't "close all tags" automatically. That often produces broken HTML. Not sure what's worse: leaving a tag open because the user forgot to close it or inserting a close tag in the wrong place.

Replying to azaozz:

Imho we shouldn't "close all tags" automatically.

Agreed. I guess I wasn't suggesting that :) Was trying to find a proper way to clear button states.

Replying to SergeyBiryukov:

Was trying to find a proper way to clear button states.

Yes, and I almost committed the patch (works well btw). But then ran some tests on the edit post screen and the HTML was "more" broken if we force-closed all tags at the end.