Opened 9 years ago

Closed 9 years ago

#3657 closed Bug (fixed)

'Select All' result in content change

Reported by: Garry Yao Owned by: Garry Yao
Priority: Must have (possibly next milestone) Milestone: CKEditor 3.0
Component: Core : Styles Version:
Keywords: Confirmed Review+ Cc:

Description

Reproducing Procedures

  1. Open the replace by class example page in FF with the following content;
    <ul><li>item</li></ul>
    
  2. Click on 'Select All' button, after this step, the selection now is actually as:
    ^<ul><li>item</li></ul>^
    
    • Expected Result: The document content remain unchanged.
    • Actual Result: There's an paragraph established before the list:
      <p>
      	<br />
      </p>
      <ul>
      	<li>
      		item</li>
      </ul>
      

Attachments (5)

3657.patch (796 bytes) - added by Garry Yao 9 years ago.
3657_2.patch (1.9 KB) - added by Garry Yao 9 years ago.
3657_3.patch (1.7 KB) - added by Garry Yao 9 years ago.
3657_4.patch (2.1 KB) - added by Garry Yao 9 years ago.
3657_5.patch (1.8 KB) - added by Garry Yao 9 years ago.

Download all attachments as: .zip

Change History (15)

Changed 9 years ago by Garry Yao

Attachment: 3657.patch added

comment:1 Changed 9 years ago by Garry Yao

Keywords: Review? added
Status: newassigned

Regression bug of [3549], adding the 'last path element is not block also' condition.

comment:2 Changed 9 years ago by Garry Yao

Adding a new patch since it's possible in IE to have the following range where the previous patch doesn't covered:

<p>paragraph1</p>
^
<p></p>

comment:3 in reply to:  2 Changed 9 years ago by Garry Yao

Replying to garry.yao:

Adding a new patch ...

After some investigation, fixing this issue will break other more general TC, so the original patch is preferred.

Changed 9 years ago by Garry Yao

Attachment: 3657_2.patch added

comment:4 Changed 9 years ago by Garry Yao

Adding a new patch to support yet another interesting TC which has been found when investigating #3671, here the selected element become a fake, block-like element like "Page Break", we can distinguish it with function CKEDITOR.dom.element::isBlockBoundary, but I'm proposing to rename this function to avoid ambiguity.

[
<img ... / > //This is a 'block-like' fake element
]

comment:5 Changed 9 years ago by Garry Yao

Another new patch with two updates;

  1. use CKEDITOR.dom.selection::getSelectedElement instead of path.lastElement to check the selected element;
  2. Free the padding block for table in IE to avoid DUP with the IE native fix.

Changed 9 years ago by Garry Yao

Attachment: 3657_3.patch added

comment:6 Changed 9 years ago by Garry Yao

Two more updates:

  1. Padding block for table is even not necessary for Webkit!
  2. The correct checking should be attribute '_cke_bogus' on L140.

Changed 9 years ago by Garry Yao

Attachment: 3657_4.patch added

comment:7 Changed 9 years ago by Frederico Caldeira Knabben

Keywords: Review- added; Review? removed

I think a simpler fix could be available. Actually, the fixing code needs to be execute exclusively over collapsed selections, so i should be enough to simply check for it.

comment:8 Changed 9 years ago by Garry Yao

Keywords: Review? added; Review- removed

Changed 9 years ago by Garry Yao

Attachment: 3657_5.patch added

comment:9 Changed 9 years ago by Frederico Caldeira Knabben

Keywords: Review+ added; Review? removed

comment:10 Changed 9 years ago by Garry Yao

Resolution: fixed
Status: assignedclosed

Fixed with [3656].

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