Opened 8 years ago

Closed 6 years ago

#6738 closed Bug (expired)

Inconsistent getCommonAncestor, need for shrink function?

Reported by: Dinu Owned by:
Priority: Normal Milestone:
Component: General Version:
Keywords: Cc:

Description

  • CKEDITOR.dom.selection.getStartElement, modifies the range; this results in that getCommonAncestor can produce different results whether it's called before or after getStartElement; it is very unusual for a getter to modify data other getters use.
  • For #6735 to work properly, it might be needed to shrink the selection like #3231, but at both ends
  • The following approach would satisfy all #3231 , #3950 and #6735 in an elegant way: create a function that shrinks the selection starting with the end, then, if not yet collapsed, from the start. This would eliminate the need for the "only blocks" exception, since the selection would be collapsed at the start in the shrink-end phase in the </td><td> case, rather than being collapsed at the end as it happens now.

Change History (5)

comment:1 Changed 8 years ago by Garry Yao

Status: newpending

selection::getStartElement doesn't actually modify the editor selection range, could you provide an example (code) by saying it affects selection::getCommonAncestor?

comment:2 Changed 8 years ago by Dinu

I think range.setStartAfter( startContainer ); does modify the range, unless it's dead code or ranges caching doesn't work. I was wrong however, it shouldn't affect getCommonAncestor, but it does affect my code (checkIsReadOnly) by moving the startContainer. Consider:

<p><span contenteditable="false">foo[</span>bar]</p>

checkIsReadOnly():true

getStartElement:

<p><span contenteditable="false">foo</span>[bar]</p>

checkIsReadOnly():false

I'll try to produce some code

comment:3 Changed 8 years ago by Garry Yao

Can we close this one by having the check read only fix at #6735?

comment:4 Changed 8 years ago by Dinu

Yes and no, #6735 doesn't address

<p><span contenteditable="false">foo[</span>bar]</p>

So in certain circumstances we won't be able to edit the start of an editable range. Haven't had time to find a test case, but as long as #3231 is still valid so should be this.

comment:5 Changed 6 years ago by Piotrek Koszuliński

Resolution: expired
Status: pendingclosed

The selection::getStartElement method was actually modifying range (by calling shrink). I fixed this when working on #9764.

Closing this issue as expired.

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