WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 17 months ago

#19389 closed defect (bug) (fixed)

Subsequent image insertion after captioned image causes breakage

Reported by: hew Owned by: azaozz
Milestone: 3.5 Priority: normal
Severity: normal Version: 2.7
Component: TinyMCE Keywords: needs-testing
Focuses: Cc:

Description

Repro (trunk):

  1. Insert an image with a caption. The Visual Editor should now report a path like this:
    Path: div » dl.wp-caption alignnone » dd.wp-caption-dd
    
  1. Immediately click "Add Media" and insert another image again (don't move cursor or anything else) with another caption. The insertion should mangle the previous image and leave a path like this in the Visual Editor:
    Path: div » dl.wp-caption alignnone » dd.wp-caption-dd » div » dl.wp-caption alignnone » dd.wp-caption-dd
    

Bonus: Enter can be pressed in the Visual Editor after step #1. That inserts a new paragraph as expected with the Visual Editor reporting "Path: p". Toggle to the HTML editor and then back to the Visual Editor. The caption element is now active (Path: div » dl.wp-caption alignnone » dd.wp-caption-dd) and an insertion at this point will cause chaos (step #2).

Repro (3.2.1):
Same as trunk except that the "Bonus" (toggling to HTML and back to Visual) does not apply.

Expected behavior:
That a subsequent image insertion would be cleanly inserted without requiring manual cursor movement.

Attachments (2)

19389.patch (2.9 KB) - added by azaozz 22 months ago.
19389-2.patch (944 bytes) - added by azaozz 17 months ago.

Download all attachments as: .zip

Change History (18)

comment:1 nacin2 years ago

  • Version set to 3.2.1

Not much worse in trunk, setting to 3.2.1.

comment:3 azaozz2 years ago

  • Milestone changed from Awaiting Review to Future Release
  • Version changed from 3.2.1 to 2.7

This has been around since image captions were introduced back in 2.7. The problem is that to place the caret in contentEditable mode the browsers need a block level element with some sort of content in it. FF has <br data-mce-bogus="1">, WebKit has U+00A0 (UTF for non-breaking space), etc. So for placing the caret we would need to insert something like <p><br data-mce-bogus="1"></p> or <p>u\00a0</p> after the caption which brings other problems when another caption is not inserted in the editor straight away.

The current behavior is a compromise. On the other hand we probably can re-examine the process of inserting a shortcode in TinyMCE and converting it to a bunch of DOM elements as the browsers have evolved a bit in that area since the last time.

comment:4 follow-up: hew2 years ago

I consider myself rather careful yet I inadvertently encountered this problem multiple times trying to arrange some screenshots in a post. It can be pretty painful as a user if the situation arises. The re-examination idea sounds neat.

comment:5 in reply to: ↑ 4 azaozz2 years ago

Replying to hew:

We can also make the whole image caption <dl> non-editable in the editor. That will prevent inserting anything where it doesn't belong but will also prevent editing the actual caption text which is possible at the moment. This had some problems when trying to drag the image before, seems to be working better now. Still needs some serious testing for all possible actions in all browsers though.

comment:6 DrewAPicture2 years ago

  • Cc xoodrew@… added

comment:8 azaozz22 months ago

  • Keywords needs-patch added
  • Milestone changed from Future Release to 3.5
  • Owner set to azaozz
  • Status changed from new to reviewing

Will make a patch that adds another <p> after each caption (as described in comment 3) so this can be tested in action.

Could also be fixed by looking at the caret position before inserting a caption. If it's already in a caption, can try to move it after.

azaozz22 months ago

comment:9 azaozz22 months ago

  • Keywords needs-testing added; needs-patch removed

comment:10 azaozz18 months ago

  • Milestone 3.5 deleted
  • Resolution set to duplicate
  • Status changed from reviewing to closed

This is handled in the new media workflow. Closing as a subset of #21390.

comment:11 follow-up: nacin17 months ago

  • Milestone set to 3.5

Worth pursuing?

comment:12 nacin17 months ago

  • Resolution duplicate deleted
  • Status changed from closed to reopened

comment:13 in reply to: ↑ 11 azaozz17 months ago

Replying to nacin:

Yes, been testing few things that would fix this and any 'send_to_editor()' while a captioned image is selected. Will have updated patch later today.

comment:14 trepmal17 months ago

#22457 was marked as a duplicate.

azaozz17 months ago

comment:15 azaozz17 months ago

19389-2.patch works well in all supported browsers. Couldn't find any side effects/interactions with other functionality too so thinking to commit it for further testing.

comment:16 azaozz17 months ago

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

In 22644:

TinyMCE: place the caret after/under images with captions when inserting content, fixes #19389

Note: See TracTickets for help on using tickets.