Fix void node clipboard event handling in Firefox #4775

skogsmaskin · 2022-01-07T11:42:36Z

Description
Firefox will not trig native clipboard events when selecting single void nodes in slate-react.

This will make sure that the DOM range is set to 0-1 (not 1-1) offset so something is selected if this is the case.

This obviously works fine in other browsers, but Firefox seems to need a real range here.

Issue
Fixes #4513

Context
This obviously works, but I'm not 100% certain there will be no side effects. Please help me think.

Also, what is strange it that it's somehow working on the Images example without this fix (and after). But I never get it to work in my code, like others have struggled with too as seen by: #4513

Checks

The new code matches the existing patterns and styles.
The tests pass with yarn test.
The linter passes with yarn lint. (Fix errors with yarn fix.)
The relevant examples still work. (Run examples with yarn start.)
You've added a changeset if changing functionality. (Add one with yarn changeset add.)

changeset-bot · 2022-01-07T11:58:22Z

⚠️ No Changeset found

Latest commit: a7a2a30

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

skogsmaskin · 2022-01-07T14:47:36Z

Found one side-effect. Arrowing up and down between void blocks now takes two key-presses.

I think it's because we don't handle up/down events in a consistent way, but leaves that up to the default browser behaviour.

Firefox seems to handle this a bit differently than the other browsers.

I've made a new commit that handles these events over void blocks consistently.

dylans

Looks great, thanks for fixing this! If you have time please add a changeset. I’ll leave it open for a couple of days for feedback and then land it unless there are objections.

skogsmaskin · 2022-01-07T17:04:11Z

@dylans - Updated the first commit to be even more agressive, checking for focus on a void block all together. Now it's working with selection spans as well (where it ends in a void node).

Will add changeset. Thanks for reviewing!

COMPAT: Firefox will not trig clipboard events when selections ends in void nodes. Make sure that the range has 0-1 (not 1-1) offset so something is selected. This obviously works fine in other browsers. Related to the zero-value (\uFEFF)?

bryanph · 2022-01-09T01:19:26Z

I think it would be a good idea to first come up with a reproducible example that consistently demonstrates this issue. I haven't been able to yet (my failed attempts in #4513 (comment)) and since the void image example works fine we might be looking at the wrong source for this? I don't think it's general behavior in firefox that a collapsed selection does not trigger an oncopy event, I've been able to confirm that much in the codesandbox at least.

Handle up down key navigation over void nodes with this specific behaviour. In this way we have a consistent way of handling these events, and it will not be up to the default browser behaviour to move the cursor. Firefox seems to handle this a bit differently than the other browsers. After doing e96a833 you had to press up/down twice to move the focus up/down between void blocks.

skogsmaskin · 2022-01-10T08:49:42Z

Extend selection up/down should also be handled in a consistent way, added that.

@bryanph - please note that the missing clipboard events in FF and this fix is only fixing the issue when the focus is at a void node (where it's consistently failing to fire those events). Might what you are looking at in the sandbox be another issue?
I agree we should put up a sandbox that shows the issue. Will try to do so.

bryanph · 2022-01-10T12:52:08Z

@skogsmaskin ah yeah that might be it, I didn't try to isolate to just void node selections. Although that leaves the question of why it does work in the image example. Perhaps some user-select: none setting?

Edit: after trying this again in my editor I can confirm that the onCopy works inconsistently and onCut does not work in my case (both work in chrome)

skogsmaskin · 2022-01-11T12:53:41Z

I've made this sandbox that illustrates the problem:

https://codesandbox.io/s/slate-react-17-oncopy-bug-not-working-forked-9n9x9?file=/index.js

I see there are some issues still which the PR doesn't solve when testing against the above code example. I think this needs to be investigated further.

dylans · 2022-01-11T13:58:05Z

I've made this sandbox that illustrates the problem:

https://codesandbox.io/s/slate-react-17-oncopy-bug-not-working-forked-0uyde?file=/index.js

I see there are some issues still which the PR doesn't solve when testing against the above code example. I think this needs to be investigated further.

https://codesandbox.io/s/slate-react-17-oncopy-bug-not-working-forked-qg78q?file=/index.js has a couple small fixes to your sandbox.

I'll leave this open for now given your comment on needing to investigate this further.

skogsmaskin · 2022-01-11T14:29:30Z

@dylans - sorry I forgot to save it! Now it is there with the latest slate-react

Right one is: https://codesandbox.io/s/slate-react-17-oncopy-bug-not-working-forked-9n9x9?file=/index.js

Could you re-apply any changes you did please?

skogsmaskin · 2022-01-11T14:45:31Z

In the sandbox: if one make a shift-arrow-right when a void (only) is selected it will fire onCut events. The selection in slate ramains the same

bryanph · 2022-01-11T16:33:31Z

Updated your example a bit to remove some errors https://codesandbox.io/s/slate-react-17-oncopy-bug-not-working-forked-mow9x?file=/index.js.

That seems like a decent reproduction, I'll try and see if the same happens in plain contenteditable

bryanph · 2022-01-11T16:43:49Z

In this very simple contenteditable example I also can't get onCut to trigger

Works in react 16: https://codesandbox.io/s/contenteditable-cut-react-16-hnbsd
Not working in react 17: https://codesandbox.io/s/contenteditable-cut-react-17-8qgtt?file=/src/App.js:310-387

So seems like a regression in react 17

I made an issue in the react repo: facebook/react#23094

skogsmaskin · 2022-01-21T09:24:49Z

Put this back to draft. Not sure if we should try to work around this or wait for React to fix the bug. So far there's not response in the issue that @bryanph filed with them.

bryanph · 2022-01-21T09:51:21Z

I can imagine it will take a while to fix on the react side, also who knows it might be some firefox quirk that is hard to resolve. So overall I'd be a proponent of a workaround. Would be nice to get a response from the react team on what the underlying cause might be though, can maybe bump the thread once in a while.

Thinking about the issue a little more I just realized that in react 17 the event handlers are no longer attached to the document root. So I tried to see if it worked if you attach an event to the document root and voila it seems to work, see https://codesandbox.io/s/contenteditable-cut-react-18-working-drqnl?file=/src/App.tsx:179-188

So I guess all we have to do is attach a cut listener to document that doesn't do anything, in that case the onCut event is also fired on the Editable div

dylans · 2022-01-22T21:49:32Z

I'd suggest that we fix per @bryanph's most recent note and keep an eye on the React issue.

skogsmaskin · 2022-01-27T09:49:13Z

Yes, this seems like the way to go! Thanks @bryanph!

dylans · 2023-12-16T11:01:54Z

@bryanph Now that we require React 18, perhaps this can be resolved?

Repository owner deleted a comment from changeset-bot bot Jan 7, 2022

skogsmaskin changed the title ~~Fix broken single-void-selections in Firefox~~ Fix broken clipboard event handling in Firefox Jan 7, 2022

skogsmaskin changed the title ~~Fix broken clipboard event handling in Firefox~~ Fix void node clipboard event handling in Firefox Jan 7, 2022

skogsmaskin force-pushed the fix/firefox-void-selections branch from 9971a42 to 02898ed Compare January 7, 2022 11:58

skogsmaskin force-pushed the fix/firefox-void-selections branch from 02898ed to e96a833 Compare January 7, 2022 12:00

skogsmaskin mentioned this pull request Jan 7, 2022

onCopy not firing for a collapsed selection using React 17 in firefox #4513

Open

skogsmaskin requested review from dylans, Soreine and tobiasandersen January 7, 2022 12:08

skogsmaskin marked this pull request as ready for review January 7, 2022 12:26

dylans approved these changes Jan 7, 2022

View reviewed changes

skogsmaskin force-pushed the fix/firefox-void-selections branch from ec47e36 to 1712d1f Compare January 7, 2022 17:02

skogsmaskin force-pushed the fix/firefox-void-selections branch from 1712d1f to eb3d35f Compare January 7, 2022 17:06

skogsmaskin force-pushed the fix/firefox-void-selections branch from eb3d35f to 82e9b9e Compare January 7, 2022 17:32

skogsmaskin force-pushed the fix/firefox-void-selections branch from 82e9b9e to a7a2a30 Compare January 10, 2022 08:42

skogsmaskin marked this pull request as draft January 21, 2022 09:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix void node clipboard event handling in Firefox #4775

Fix void node clipboard event handling in Firefox #4775

skogsmaskin commented Jan 7, 2022 •

edited

changeset-bot bot commented Jan 7, 2022 •

edited

skogsmaskin commented Jan 7, 2022 •

edited

dylans left a comment

skogsmaskin commented Jan 7, 2022 •

edited

bryanph commented Jan 9, 2022

skogsmaskin commented Jan 10, 2022 •

edited

bryanph commented Jan 10, 2022 •

edited

skogsmaskin commented Jan 11, 2022 •

edited

dylans commented Jan 11, 2022

skogsmaskin commented Jan 11, 2022 •

edited

skogsmaskin commented Jan 11, 2022 •

edited

bryanph commented Jan 11, 2022

bryanph commented Jan 11, 2022 •

edited

skogsmaskin commented Jan 21, 2022

bryanph commented Jan 21, 2022

dylans commented Jan 22, 2022

skogsmaskin commented Jan 27, 2022

dylans commented Dec 16, 2023

Fix void node clipboard event handling in Firefox #4775

Are you sure you want to change the base?

Fix void node clipboard event handling in Firefox #4775

Conversation

skogsmaskin commented Jan 7, 2022 • edited

changeset-bot bot commented Jan 7, 2022 • edited

⚠️ No Changeset found

skogsmaskin commented Jan 7, 2022 • edited

dylans left a comment

Choose a reason for hiding this comment

skogsmaskin commented Jan 7, 2022 • edited

bryanph commented Jan 9, 2022

skogsmaskin commented Jan 10, 2022 • edited

bryanph commented Jan 10, 2022 • edited

skogsmaskin commented Jan 11, 2022 • edited

dylans commented Jan 11, 2022

skogsmaskin commented Jan 11, 2022 • edited

skogsmaskin commented Jan 11, 2022 • edited

bryanph commented Jan 11, 2022

bryanph commented Jan 11, 2022 • edited

skogsmaskin commented Jan 21, 2022

bryanph commented Jan 21, 2022

dylans commented Jan 22, 2022

skogsmaskin commented Jan 27, 2022

dylans commented Dec 16, 2023

skogsmaskin commented Jan 7, 2022 •

edited

changeset-bot bot commented Jan 7, 2022 •

edited

skogsmaskin commented Jan 7, 2022 •

edited

skogsmaskin commented Jan 7, 2022 •

edited

skogsmaskin commented Jan 10, 2022 •

edited

bryanph commented Jan 10, 2022 •

edited

skogsmaskin commented Jan 11, 2022 •

edited

skogsmaskin commented Jan 11, 2022 •

edited

skogsmaskin commented Jan 11, 2022 •

edited

bryanph commented Jan 11, 2022 •

edited