You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The link highlight is a ck-link_selected class added to a link element. It's an attribute so it should be added by the renderer._updateAttrs() method. This happens in most cases, for example, when moving caret like:
renderer adds highlight class via renderer._updateAttrs() method (as a element was marked to have its attributes updated) and then the renderer._updateChildren() method is run. Since nothing has changed, DOM structure is not modified.
When the caret is placed on the beginning of a link (within text) like:
Children of the p element are updated in renderer._updateChildren(). Since a view element is different, its DOM representation gets rerendered (old a is replaced with new a.ck-link_selected).
Now renderer._updateChildren() is executed for the old a view element. Since it has no parent and cannot be mapped to the DOM, nothing changes.
So here the fact that link is highlighted is simply caused by the fact that a element was replaced, not because its attributes were updated (as it should happen IMHO).
Here situation is similar but with additional renderer._updateAttrs() call.
The a elements is marked to have its attributes updated (renderer.markedAttributes)1.
The p and a elements are marked to be updated (renderer.markedChildren)1.
The attributes of a are updated in the renderer._updateAttrs() method. Highlight ck-link_selected class is removed.
Children of the p element are updated in renderer._updateChildren(). Since a view element is different, its DOM representation gets rerendered (old a is replaced with new a.ck-link_selected).
Now renderer._updateChildren() is executed for the old a view element. Since it has no parent and cannot be mapped to the DOM, nothing changes.
In this case, renderer._updateAttrs() removes highlight class and then it's added because whole a element is rerenderd.
1. When a (view element) is being marked to sync it has a parent element but when rednerer.render() is called, its parent is already empty (which means it was removed from view structure and replace with another element).
I think in all the cases above, DOM a element should not be reinserted, only its attributes updated. Still I'm not sure if we should mark it as a bug or enhancement.
The text was updated successfully, but these errors were encountered:
Definitely. We should only touch this element's attributes.
It would be good to understand why p's children are marked to be rendered. Most likely, unfortunately, due to selection's conversion. That may be again the same story that the selection is converted next to the existing <a> element so this happens for a while:
<p><a>[]</a><a>xxx</a></p>
And then merge happens... to the left side. So we lose the reference to the original <a> element (the one on the rigth hand side). And, at the same time, we mark <p> children to be re-rendered. There's a ticket in which I described this mechanism. Do you remember which was it?
Now... your PR should theoretically fix this. Why doesn't it do this? Doesn't it find the new <a> a replacement of the old <a>?
Now... your PR should theoretically fix this. Why doesn't it do this? Doesn't it find the new a replacement of the old ?
It does, but due to the fact that it rebinds mappings (so the old a view element is removed from mappings), renderer._updateAttrs which used this old a element to update attributes throws an error (becuase there is no corresponding DOM element). It looks like more generic problem with updating attributes of elements which was replaced in a view structure. I'm looking for some reasonable solution for this case now.
Fix: Renderer should avoid doing unnecessary DOM structure changes. Ensuring that the DOM gets updated less frequently fixes many issues with text composition. Closes #1417. Closes #1409. Closes #1349. Closes #1334. Closes #898. Closes ckeditor/ckeditor5-typing#129. Closes ckeditor/ckeditor5-typing#89. Closes #1427.
mlewand
transferred this issue from ckeditor/ckeditor5-engine
Oct 9, 2019
The link highlight is a
ck-link_selected
class added to a link element. It's an attribute so it should be added by therenderer._updateAttrs()
method. This happens in most cases, for example, when moving caret like:renderer
adds highlight class viarenderer._updateAttrs()
method (asa
element was marked to have its attributes updated) and then therenderer._updateChildren()
method is run. Since nothing has changed, DOM structure is not modified.When the caret is placed on the beginning of a link (within text) like:
attributes are not modified (since selection is still outside of the link), or like:
highlight class is simply removed in
renderer._updateAttrs()
method and no DOM rendering takes place.The situation is quite different when placing selection on the beginning of the link which is a first element in the paragraph. There are two cases.
Moving selection from regular text to the link:
renderer.markedAttributes
is empty).p
anda
elements are marked to be updated (renderer.markedChildren
)1.p
element are updated inrenderer._updateChildren()
. Sincea
view element is different, its DOM representation gets rerendered (olda
is replaced with newa.ck-link_selected
).renderer._updateChildren()
is executed for the olda
view element. Since it has no parent and cannot be mapped to the DOM, nothing changes.So here the fact that link is highlighted is simply caused by the fact that
a
element was replaced, not because its attributes were updated (as it should happen IMHO).Moving selection within the link:
Here situation is similar but with additional
renderer._updateAttrs()
call.a
elements is marked to have its attributes updated (renderer.markedAttributes
)1.p
anda
elements are marked to be updated (renderer.markedChildren
)1.a
are updated in therenderer._updateAttrs()
method. Highlight ck-link_selected class is removed.p
element are updated inrenderer._updateChildren()
. Sincea
view element is different, its DOM representation gets rerendered (olda
is replaced with newa.ck-link_selected
).renderer._updateChildren()
is executed for the olda
view element. Since it has no parent and cannot be mapped to the DOM, nothing changes.In this case,
renderer._updateAttrs()
removes highlight class and then it's added because wholea
element is rerenderd.1. When
a
(view element) is being marked to sync it has aparent
element but whenrednerer.render()
is called, its parent is already empty (which means it was removed from view structure and replace with another element).I think in all the cases above, DOM
a
element should not be reinserted, only its attributes updated. Still I'm not sure if we should mark it as a bug or enhancement.The text was updated successfully, but these errors were encountered: