Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

InputCommand loses selection attributes when selection is anchored to attribute beginning #7816

Open
mlewand opened this issue Aug 10, 2020 · 5 comments
Labels
bc:minor Resolving this issue will introduce a minor breaking change. domain:ui/ux This issue reports a problem related to UI or UX. package:typing squad:core Issue to be handled by the Core team. support:2 An issue reported by a commercially licensed client. type:bug This issue reports a buggy (incorrect) behavior.

Comments

@mlewand
Copy link
Contributor

mlewand commented Aug 10, 2020

📝 Provide detailed reproduction steps (if any)

  1. Open the classic editor example.
  2. Select few words, e.g. "things on earth" and bold it.
  3. Select word "things".

  1. Execute editor.execute( 'input', { text: 'foobar' } ); in your JavaScript console.

✔️ Expected result

The inserted text keeps original styles (in this case bold).

❌ Actual result

The replaced part is not bold.

📃 Other details

This is a source of WebSpellChecker/wproofreader-ckeditor5#12 - as WSC team is using editor.execute('input', { text: 'text' }); call to make text modifications.

Reason for this is that we pick attributes from a new selection, after original part was removed:

model.insertContent( writer.createText( text, doc.selection.getAttributes() ), selection );

  • Browser: Any

If you'd like to see this fixed sooner, add a 👍 reaction to this post.

@mlewand mlewand added type:bug This issue reports a buggy (incorrect) behavior. package:typing squad:red labels Aug 10, 2020
@mlewand
Copy link
Contributor Author

mlewand commented Aug 10, 2020

I have pushed i/7816 branch that contains an example fix (but no tests) for this issue.

@mlewand mlewand added the bc:minor Resolving this issue will introduce a minor breaking change. label Aug 10, 2020
@mlewand
Copy link
Contributor Author

mlewand commented Aug 10, 2020

Surprisingly very little tests have failed after the change.

I'm marking this issue as a potential breaking change, as changing it will especially affect the case where you "overwrite" the entire node with an attribute (currently replaced content is plain text, whereas now it would inherit the attributes of a replaced text).

@Reinmar
Copy link
Member

Reinmar commented Aug 10, 2020

This was working this way by design. You can observe the same behavior upon typing in CKE5.

However, I noticed that both Pages, Word, and Google Docs inherit the selection styles in this case and I'm totally fine with changing this behavior on our side too.

@Reinmar Reinmar added squad:core Issue to be handled by the Core team. domain:ui/ux This issue reports a problem related to UI or UX. and removed squad:red labels Aug 10, 2020
@mlewand
Copy link
Contributor Author

mlewand commented Aug 11, 2020

Should the type over behavior be still covered by this issue or should we extract it into a separate issue?

Reason why I am asking is that this dead simple fix that I proposed solves already the InputCommand (initial comment scope), but getting it to work with typing will be tirckier, it doesn't work out of the box. My assumption is that the mutation observers kick in late, after the initial content is removed without preserving the issue.

@Reinmar Reinmar added this to the nice-to-have milestone Aug 17, 2020
@oleq
Copy link
Member

oleq commented Jan 18, 2021

My assumption is that the mutation observers kick in late, after the initial content is removed without preserving the issue.

Once we switch to beforeinput this may change. So maybe let's wait for #8007 first?

@pomek pomek removed this from the nice-to-have milestone Feb 21, 2022
@martynawierzbicka martynawierzbicka added the support:2 An issue reported by a commercially licensed client. label Jul 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bc:minor Resolving this issue will introduce a minor breaking change. domain:ui/ux This issue reports a problem related to UI or UX. package:typing squad:core Issue to be handled by the Core team. support:2 An issue reported by a commercially licensed client. type:bug This issue reports a buggy (incorrect) behavior.
Projects
None yet
Development

No branches or pull requests

5 participants