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

Marking header row in tables act not as expected. #17962

Open
pszczesniak opened this issue Feb 20, 2025 · 3 comments · May be fixed by #17870
Open

Marking header row in tables act not as expected. #17962

pszczesniak opened this issue Feb 20, 2025 · 3 comments · May be fixed by #17870
Labels
package:table squad:core Issue to be handled by the Core team. type:bug This issue reports a buggy (incorrect) behavior.

Comments

@pszczesniak
Copy link
Contributor

pszczesniak commented Feb 20, 2025

📝 Provide detailed reproduction steps (if any)

  1. Open for example - ckeditor.com/docs/ckeditor5/latest/examples/builds-custom/full-featured-editor.html
  2. Using Source Editing apply this table:
<figure class="table"><table><tbody><tr><td>a</td><td>b</td><td>c</td></tr><tr><td>1</td><td>2</td><td>3</td></tr></tbody></table></figure>
  1. Focus any cell from first row open Row dropdown and click on Header row toggle button.
  2. Click again.

(Chrome version matters - check the Other details section below 👇)

✔️ Expected result

After first click, the focused table row should became a header row, after second click should became a regular one.

❌ Actual result

After first click, the focused table row becomes a header row, but toggle button didnt change his state, after second click the toggle button changed his state also second row is marked as a header row.

❓ Possible solution

If you have ideas, you can list them here. Otherwise, you can delete this section.

📃 Other details

⚠ It was worked ok for example on Chrome v.131, but after installed the newest version (also tested on Canary) it's not work properly anymore. ⚠


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

@pszczesniak pszczesniak added package:table squad:core Issue to be handled by the Core team. type:bug This issue reports a buggy (incorrect) behavior. labels Feb 20, 2025
@pszczesniak
Copy link
Contributor Author

After a quick debugging it looks like a Chrome started to emit selectionchange event sooner, which cause the editor to move selection.

Screen.Recording.2025-02-21.at.12.18.19.mov

Sometimes it works correctly, maybe some race conditions?

@niegowski
Copy link
Contributor

As the DOM structure changes, the DOM selection should also be updated to anchor the correct position. There is a condition before the update of the DOM selection:

if ( !this.isFocused || !domRoot ) {

I tried to modify this condition so it updates the DOM selection on non-focused editable if there were any structural changes in the document (this.markedChildren.size) but unfortunately updating the DOM selection on non-focused editable triggers the focus of that editable (it is a DOM behavior, can be reproduced without editor).

Now I'm considering some handling on the selection observer side.

@niegowski
Copy link
Contributor

Some promising results in PoC: #17870
In the PoC:

  • SelectionObserver ignores selection change events on non-focused editable (sets flag that there is a selection change pending)
  • Focus observer on timeout flush, flushes the selection observer - triggers pending selection change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:table squad:core Issue to be handled by the Core team. type:bug This issue reports a buggy (incorrect) behavior.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants