liferay / liferay-ckeditor

Other
8 stars 49 forks source link

fix: LPS-137425 Focusing on balloon editor toolbar may open an additional text toolbar #189

Closed markocikos closed 3 years ago

markocikos commented 3 years ago

Fixes: https://issues.liferay.com/browse/LPS-137425

In this PR, we are removing force selection update on focus. Selection update has a lot of side-effects, including opening a balloon toolbar. I tried multiple times to makes a workaround, see https://github.com/liferay-frontend/liferay-portal/pull/1284 and https://github.com/liferay-frontend/liferay-portal/pull/1350, but each solution leads to a new bug. I finally ran out of options, the only solution remaining to fix the root cause.

I was hesitant to do modify base ckeditor because the comment references previousActive sync. Looking in more depth in the ckeditor code, I think removing this block is safe to do:

Before PR: before

After PR: after

diegonvs commented 3 years ago

If that's the case, we will want to add a more precise solution, instead of forcing selection that has many side-effects.

I tried to see if we can event.stopPropagation() when clicking in linktoolbar's buttons but with the pattern "onclick="CKEDITOR.tools.callFunction" we don't receive event information. 😞

In overall, considering "The purpose of the removed code does not seem critical, restoring selection on blur/focus. The worst case scenario is we re-introduce a minor bug, affecting an edge case." it looks good to me.