Ticket #5309 (closed New Feature: fixed)

Opened 5 years ago

Last modified 14 months ago

Context sensitive insertHtml

Reported by: garry.yao Owned by: garry.yao
Priority: Normal Milestone:
Component: Core : Pasting Version:
Keywords: Cc:

Description (last modified by garry.yao) (diff)

We've seen various paste problems caused by our current implementation of CKEDITOR.editor::insertHtml, now rely on browsers' native API, which is buggy and inconsistent.
While we do have other options here: The idea is to implement an DTD aware, context sensitive insertion based on CKEDITOR.htmlParser' and 'CKEDITOR.editor::insertElement', this should be able to extinguish all weird bugs around.

Related tickets are:

#5696
IE : Cursor moves to the start of paragraph in the Pasted Text

#5367
Implement editor.insertText and use it in the special chars dialog

#5536
Problems with pasting (executing insertHtml) and formatted text

#5170
Problems with pasting (executing insertHtml) and formatted text

#5033
Invalid behavior when inserting nested divs with insertHtml()

#4898
Paste: After pasting a table from word it is not possible to navigate outside the table

#4746
insertHtml left trim text

#3105
insertHtml and insertElement should agree on the result

#2781
FckEditor InsertHtml inserts extra <p> tags

#2749
InsertHtml() is inconsistent between IE 7 and Firefox 2

#2526
InsertHTML Problem

#503
Unordered list paste: doesn't work correctly in IE

Attachments

5309.patch (15.4 KB) - added by garry.yao 4 years ago.
5309_2.patch (15.9 KB) - added by garry.yao 4 years ago.
5309_3.patch (16.5 KB) - added by Saare 4 years ago.
Align the outdated patch
5309_4.patch (16.4 KB) - added by garry.yao 4 years ago.
5309_5.patch (20.6 KB) - added by garry.yao 4 years ago.
5309_6.patch (20.4 KB) - added by garry.yao 4 years ago.

Change History

comment:1 Changed 5 years ago by garry.yao

  • Component changed from General to Core : Pasting
  • Description modified (diff)

comment:2 Changed 5 years ago by garry.yao

  • Description modified (diff)

Minor adjustments.

comment:3 Changed 5 years ago by garry.yao

  • Description modified (diff)

comment:4 Changed 5 years ago by garry.yao

By checking the insertHtml API in latest HTML5 draft, it also doesn't require a context sensitive insertion:

4. Invoke the HTML fragment parsing algorithm with an arbitrary orphan 'body' element owned by the same Document as the context element and with the value argument as input.

comment:5 Changed 5 years ago by garry.yao

  • Milestone changed from CKEditor 3.3 to CKEditor 3.4

We're over burdened now so this feature will be deferred to next milestone.

Changed 4 years ago by garry.yao

comment:6 Changed 4 years ago by garry.yao

  • Status changed from new to assigned
  • Keywords Confirmed Review? added; Umbrella removed
  • Type changed from Task to New Feature
  • Description modified (diff)
  • Owner set to garry.yao

Ported a simplified version of Firefox's nsHTMLEditor::InsertHTMLWithContext while it supposed to be browser neutral paste implementation.

comment:7 Changed 4 years ago by garry.yao

  • Description modified (diff)

Changed 4 years ago by garry.yao

comment:8 Changed 4 years ago by garry.yao

Align the outdated patch.

comment:9 Changed 4 years ago by wwalc

Might be a duplicate of one of the tickets, but I'm adding it just to have one more test case: #5873

comment:10 Changed 4 years ago by fredck

  • Milestone changed from CKEditor 3.4 to CKEditor 3.5

comment:11 Changed 4 years ago by fredck

  • Milestone changed from CKEditor 3.4.1 to CKEditor 3.5

Changed 4 years ago by Saare

Align the outdated patch

Changed 4 years ago by garry.yao

comment:12 Changed 4 years ago by garry.yao

Align outdated patch

Changed 4 years ago by garry.yao

comment:13 Changed 4 years ago by garry.yao

New patch with some updates:

  1. Instead of using "range::extractContent" which is currently buggy when dealing with list and table selection, we now play a browser trick (insertHtml called with an empty content) to achieve the same result;
  2. Introduce a HTML parser param (clipboard) to handle the case when parsing context of the fragment is unknown, where we should not touch the white-spaces children of the fragment.

comment:14 Changed 4 years ago by garry.yao

It looks like a huge task to make a reviewing here, so let me also propose some orders here:

  1. First we check there's no exception thrown in any case when pasting, this must be guaranteed;
  2. Then we should check paste in any case should not result in malformed structure (DTD valid);
  3. Finally it come to a case by case test with different combination of clipboard content and selection type to check the rationality of the result, in case it's amphibolous, we should allow it to be handled out of this ticket.

Changed 4 years ago by garry.yao

comment:15 Changed 4 years ago by garry.yao

Trunk alignment.

comment:16 Changed 4 years ago by Saare

  • Status changed from review to review_failed
  • Keywords Confirmed removed

Please align the patch with the latest updates (data-cke attributes).

comment:17 Changed 4 years ago by fredck

  • Milestone changed from CKEditor 3.5 to CKEditor 3.6

This one, other than introducing a good amount of code into core (justifiable), touches import part of it. We need a lot of attention on it and we'll not be able to safely land this on the SVN for the 3.5.

comment:18 Changed 4 years ago by fredck

  • Milestone CKEditor 3.6 deleted

comment:19 Changed 14 months ago by Reinmar

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

Fixed in 4.0.

Note: See TracTickets for help on using tickets.
© 2003 – 2012 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy