ianstormtaylor / slate

A completely customizable framework for building rich text editors. (Currently in beta.)
http://slatejs.org
MIT License
29.6k stars 3.23k forks source link

Chrome 105 breaks slate 0.27.x #5110

Open beaugunderson opened 2 years ago

beaugunderson commented 2 years ago

Description Our application (an electronic health record) uses slate in production; versions:

    "slate": "0.27.4",
    "slate-auto-replace": "0.8.1",
    "slate-react": "0.7.2",

The update to Chrome 105 now rolling out breaks the ability for slate to find a DOM point, it looks like.

image

In the above screenshot you can see that leaf will be null as end is zero and point.offset is 2, meaning the condition of end >= point.offset will never be satisfied.

I realize we're on an old version of slate but if there's a known fix for Chrome 105 issues or a link to crbug for whatever the regression is there we would be very grateful.

Recording Will follow up with a GIF.

Environment

beaugunderson commented 2 years ago

Sorry in advance for the stream of consciousness as I debug this...

I removed the placeholder key from our Editor component. Typing the first character results in a point with an offset of 1.

node.getLeaves() returns a list of 1 leaf, and that leaf has a text attribute of '', which has length of 0.

Because of this leaves.find(...) does not return a leaf (it's looking for the leaf that is equal to or after the point).

I think we can infer that something in Chrome 105 changed the point findPoint is returning? I will try to verify exactly what is returned on Chrome < 105.

beaugunderson commented 2 years ago

Interesting, prior to Chrome 105 onInput never triggers, only onBeforeInput does. The code that fails is not called in our workflow prior to Chrome 105.

beaugunderson commented 2 years ago

Chrome 104:

image

vs. Chrome 105 (the traceback happens immediately after the last line):

image
beaugunderson commented 2 years ago

Realized I never included the traceback above:

image
zarv1k commented 2 years ago

I'm also experiencing some critical issues in Chrome 105 that I've never seen before 105 release, using immutable.js based version of SlateJS editor (slate@0.47.4 + slate-react@0.22.4)

https://codesandbox.io/s/cfdps

  1. Try to type some word in empty editor, then move caret somewhere in the middle of your word and type some one char - your char will be added, but caret will be moved to the end of current text node, so it's impossible even to type a few chars in a row in the middle of any text node w/o caret jumping to the end.
  2. Try to press Enter/Return a few times to make a few empty paragraphs in your editor, then type just one char in any of your empty paragraph and after that try to press Backspace to remove your last char in your last typed char - it will not be deleted, because your just deleted ZWNBSP char that was added after your first char in paragraph. Slate uses ZWNBPS to make it possible to put caret into an empty paragraph, so it seems that just after you typed your first char in an empty paragraph, the ZWNBSP also added after your char.
  3. Try to type some one char in the empty editor with placeholder. Instead your placeholder hidden and just one char added into editor, the placeholder text will be added at first, followed by your char and then ZWNBSP also added as the last character, that is really strange.

I don't think it's the complete list of issues described above, just ones that I was able to reproduce and describe, so the Slate-based editor's behaviour is completely broken in Chrome 105.

None of these issues, described above, have ever been reproduced before, neither in any Chrome version prior 105 nor in any other browsers (Safari on macOS, Safari on iOS, FF on any OS, etc.).

zarv1k commented 2 years ago

@beaugunderson

OMG! I can't believe that all the issues in Chrome 105 described in my previous comment here were because of style WebkitUserModify: 'read-write-plaintext-only' on contentEditable container.

After some deep investigation, I realized that event.getTargetRanges() returns an empty array in onBeforeInput handler in after plugin when that WebkitUserModify style applied. In this case onBeforeInput handler returns and does nothing, so that you can see onInput event handler works instead as a fallback that leads to some unexpected behaviour in my cases.

Please check it out for your case - it might be the root of the issue you are experiencing.

eMerzh commented 2 years ago

it's related to #5108 no?

zarv1k commented 2 years ago

it's related to #5108 no?

https://github.com/ianstormtaylor/slate/issues/5108#issuecomment-1235137908

fikrikarim commented 2 years ago

@beaugunderson

OMG! I can't believe that all the issues in Chrome 105 described in my previous comment here were because of style WebkitUserModify: 'read-write-plaintext-only' on contentEditable container.

After some deep investigation, I realized that event.getTargetRanges() returns an empty array in onBeforeInput handler in after plugin when that WebkitUserModify style applied. In this case onBeforeInput handler returns and does nothing, so that you can see onInput event handler works instead as a fallback that leads to some unexpected behaviour in my cases.

Please check it out for your case - it might be the root of the issue you are experiencing.

Hi @zarv1k. We're experiencing the exact same problems that you mentioned in the previous comment. And changing the style on contentEditable from WebkitUserModify: 'read-write-plaintext-only' to WebkitUserModify: 'read-write' fix every issue we got so far.

I'm wondering if this is the best workaround for now, or do you have a better way to solve this?

zarv1k commented 2 years ago

@beaugunderson

OMG! I can't believe that all the issues in Chrome 105 described in my previous comment here were because of style WebkitUserModify: 'read-write-plaintext-only' on contentEditable container.

After some deep investigation, I realized that event.getTargetRanges() returns an empty array in onBeforeInput handler in after plugin when that WebkitUserModify style applied. In this case onBeforeInput handler returns and does nothing, so that you can see onInput event handler works instead as a fallback that leads to some unexpected behaviour in my cases.

Please check it out for your case - it might be the root of the issue you are experiencing.

Hi @zarv1k. We're experiencing the exact same problems that you mentioned in the previous comment. And changing the style on contentEditable from WebkitUserModify: 'read-write-plaintext-only' to WebkitUserModify: 'read-write' fix every issue we got so far.

I'm wondering if this is the best workaround for now, or do you have a better way to solve this?

According to comment in sources this style was added just to hide b/i/u menu on iOS (it also disables the same menu items in context menu Font -> Bold etc... on Safari desktop). I'm not sure about Chrome mobile (has no device to check), but you definitely could remove this style completely for Chrome desktop

dylans commented 2 years ago

Note this does not appear to be an issue with any recent versions of Slate, only with versions released more than 3 years ago. We'll leave this open so you can sort through it, but Slate won't be releasing an update to a version this far back.

fikrikarim commented 2 years ago

Just want to add that it's also happening on 0.47.8. So it might be any versions before v0.50+.

dylans commented 2 years ago

Just want to add that it's also happening on 0.47.8. So it might be any versions before v0.50+.

FWIW, anything before 0.59 is ~2.5 to 3 years old at this point.

beaugunderson commented 2 years ago

FWIW, anything before 0.59 is ~2.5 to 3 years old at this point.

Point definitely taken :) I appreciate you leaving this open so people still on the old versions can figure out a workaround until we can upgrade (or more likely rewrite in our case since so much has changed) 🙏

Taelar commented 2 years ago

Hi thanks for this thread, it's very helpful :) Any new workaround found ? We're experiencing big issues on our side as well, especially on production builds (Slate 0.47)

zarv1k commented 2 years ago

@ThomasEsseul Why some another workaround needed? Didn't this help you? - https://github.com/ianstormtaylor/slate/issues/5110#issuecomment-1235150979

I have no issues in Chrome 105 using Slate 0.47.8 after I removed WebkitUserModify: 'read-write-plaintext-only' completely. But I kept it for Safari iOS, Chrome Mobile on iOS and Safari on Mac, coz it hides BIU in context menu which doesn't work in sync with Slate.

Taelar commented 2 years ago

@zarv1k It does help yes ! I was just wondering if there were any new informations on this. I think I will go for this solution then, thanks :)

piciuok commented 2 years ago

We have same problem with version:

We have some schema validation as below:

// common schema validation for cloud nodes
const createCloudValidation = nodeType => ({
  // check if cloud node text
  // has whitespace at the beginning and the end
  text: txt => {
    const isFirstCharValid = txt[0] === ' ';
    const isLastCharValid = txt.slice(-1) === ' ';
    const isLengthValid = txt.length > 1;

    return isFirstCharValid && isLastCharValid && isLengthValid;
  },
  normalize: (editor, { code, node }) => {
    const nodeText = node.text;

    if (code === 'node_text_invalid') {
      if (nodeText.length === 1) {
        editor.unwrapInline(nodeType);

        return;
      }

      const isFirstCharValid = nodeText[0] === ' ';
      if (!isFirstCharValid) {
        // find first text node
        const firstTextNode = node.getFirstText();

        // append space on the beginning of text
        editor.insertTextByKey(firstTextNode.key, 0, ' ');

        return;
      }

      const isLastCharValid = nodeText.slice(-1) === ' ';
      if (!isLastCharValid) {
        // find last text node
        const lastTextNode = node.getLastText();
        const textLength = lastTextNode.text.length;

        // append space on the end of text
        editor.insertTextByKey(lastTextNode.key, textLength, ' ').moveBackward();
      }
    }
  },
});

Firstly, when we add node with 'cloud' type everything works great, but when i want to change text inside, text goes at beginning of whole document and node disappear. While debugging i noticed that text method of validator is called twice - first call with empty string, second call with actual content so validator fail.

If i removed unwrapInline, all works seems to works great but validation fire twice so every typed char it added me spaces at end... ;d

I think we should plan complety rewrite editor to latest version 😄

swnorowski commented 2 years ago

My team found that in previous versions of Chrome (before 105), the event fired was actually a React synthetic event. We spent a while trying to debug what changed with event.getTargetRanges() but ultimately what changed is that we were calling that function at all.

We quickly fixed our prod issues by, for now, forking our version of slate and returning early if we are in in Chromium 105+, but we may explore the WebkitUserModify fix as well.

Taelar commented 2 years ago

My team published a hotfix with the WebkitUserModify solution, it seems to work fine ! But we have some users (Mac + Chrome 105+ users only) who experience problems when typing some specific special characters (i.e. ê, â, ï, ô... 🥖). Did anyone hear about such a case on their side ?

roman-korovets commented 2 years ago

@beaugunderson

OMG! I can't believe that all the issues in Chrome 105 described in my previous comment here were because of style WebkitUserModify: 'read-write-plaintext-only' on contentEditable container.

Thank you.

Code like this works for me: #editor-id:not(.disabled) { -webkit-user-modify: read-write; }

beaugunderson commented 2 years ago

Back from out of internet range and can confirm that the -webkit-user-modify fix does work for us as well. Thank you everyone for chiming in with help. 🙏

CodyJamesCasey commented 2 years ago

Just chiming in - my team is at the tail end of a rewrite on the latest and greatest, but our production app is facing this issue as well.

"slate": "npm:slate@^0.39.3",
"slate-react": "npm:slate-react@^0.17.3",

So far, the WebkitUserModify isn't working for us at all. Now I'm trying to explore the package resolution workaround suggested in the other open issue.

CodyJamesCasey commented 2 years ago

So far, the WebkitUserModify isn't working for us at all. Now I'm trying to explore the package resolution workaround suggested in https://github.com/ianstormtaylor/slate/issues/5108.

The fix to resolve to an earlier version of slate-dev-environment worked for us 🚀

  "resolutions": {
    "slate-dev-environment": "0.1.4"
  },
Nantris commented 2 years ago

It seems like between the two workarounds mentioned in this issue, everyone should be able to workaround this issue.

Is anyone not able to get their editor working with the provided workarounds?

dennisrcao commented 1 year ago

@ThomasEsseul Why some another workaround needed? Didn't this help you? - #5110 (comment)

I have no issues in Chrome 105 using Slate 0.47.8 after I removed WebkitUserModify: 'read-write-plaintext-only' completely. But I kept it for Safari iOS, Chrome Mobile on iOS and Safari on Mac, coz it hides BIU in context menu which doesn't work in sync with Slate.

Thank you for pointing this out. I was seeing the following bugs on slate 0.47

As well as importing IS_IOS, IS_SAFARI, (line 13 and 14)

Hope that helps someone.

stowball commented 1 year ago

If you want a CSS only solution to targeting Blink-based browsers (i.e. not Safari), you can use:

@supports not (-apple-trailing-word: inherit) {
  div[data-slate-editor] {
    -webkit-user-modify: read-write !important;
  }
}
bobey commented 1 year ago

@ThomasEsseul

My team published a hotfix with the WebkitUserModify solution, it seems to work fine ! But we have some users (Mac + Chrome 105+ users only) who experience problems when typing some specific special characters (i.e. ê, â, ï, ô... 🥖). Did anyone hear about such a case on their side ?

Yes, we meet the exact same problem with ê, â ... characters.

@Slapbox

Is anyone not able to get their editor working with the provided workarounds?

We applied the -webkit-user-modify fix but still have problems with accented characters.

Does anyone knows a fix around this particular issue?

Nantris commented 1 year ago

@bobey did you try forcing resolution of slate-dev-environment to an older version as mentioned by @CodyJamesCasey?

Note that the syntax @CodyJamesCasey provided is for yarn, but this can be achieved with npm/pnpm as well with the correct syntax.

bobey commented 1 year ago

@bobey did you try forcing resolution of slate-dev-environment to an older version as mentioned by @CodyJamesCasey?

Note that the syntax @CodyJamesCasey provided is for yarn, but this can be achieved with npm/pnpm as well with the correct syntax.

Hi @Slapbox, thanks for your help on this. I tried indeed but I get a compilation error as my slate version seems to try loading a constant which is not exported anymore:

./.yarn/__virtual__/slate-react-virtual-956c00d07a/0/cache/slate-react-npm-0.22.4-d1696dbb04-f463ef2279.zip/node_modules/slate-react/lib/slate-react.es.js
Attempted import error: 'ANDROID_API_VERSION' is not exported from 'slate-dev-environment'.

Am I missing something here?

We're using the following versions ATM:

{
  "dependencies": {
    "slate": "0.47.4",
    "slate-html-serializer": "0.8.6",
    "slate-plain-serializer": "0.7.6",
    "slate-react": "0.22.4",
    "...": "..."
  },
  "resolutions": {
    "slate-dev-environment": "0.1.6"
  }
}
Nantris commented 1 year ago

If you use Webpack you could try using ProvidePlugin, I think it is, to provide that value - but I don't know if it would work so it may not be worth your time.

mathisobadia commented 1 year ago

Ok So I thought I would explain the way I fixed this bug because some of the provided solutions did not work for us: -webkit-user-modify solution did fix the cursor thing but broke everything else. Changing the resolutions so an old slate-dev-environment didn't work either as it broke the build.

What did work was:

  1. install patch-package and do the required changes to your package.json to make it work
  2. create a file named slate-react+0.22.10.patch in a folder named 'patches' placed at the root of your project
  3. put this code in the newly created file:
    
    diff --git a/node_modules/slate-react/lib/slate-react.es.js b/node_modules/slate-react/lib/slate-react.es.js
    index cfb874f..af063a2 100644
    --- a/node_modules/slate-react/lib/slate-react.es.js
    +++ b/node_modules/slate-react/lib/slate-react.es.js
    @@ -4021,7 +4021,8 @@ function AfterPlugin() {
    function onBeforeInput(event, editor, next) {
     var value = editor.value;

Anyway it's really a pain to have to work on legacy code has not been upgraded since 3 years but it is what it is.

bobey commented 1 year ago

Hi all,

I'd like to thank everyone here who shared feedbacks and tips to help fix this issue.

Just to let you know that we applied all options shared here, even @mathisobadia's one, it fixes part of the bug on our side but we still have a failure with accented characters.

If someone discover a fix for this particular issue, do not hesitate to share it 🙏

nbarrera commented 1 year ago

Hi there... just in case it helps anyone the way we fixed this was to execute this code on a <head> <script>

delete HTMLElement.prototype.onbeforeinput

on Google Chrome 105+ they added a new way of setting the beforeinput event, now you can also set is as a DOM element attribute (just like <a onclick="" />)

so precisely, slate was detecting on which input event level the browser is at... by detecting if any given DOM element supports the beforeinput attribute.

So with this fix... we make slate think that Chrome 105+ and Chrome < 105 are in the same input event level

there is a side effect which is that you won't be able to define a beforeinput event as a dom element attribute but.. well :shrug:

Nantris commented 1 year ago

For anyone curious, the problem code in slate-dev-environment (from the compiled code) looks like this:

var FEATURE_RULES = [['inputeventslevel1', function (window) {
  var event = window.InputEvent ? new window.InputEvent('input') : {};
  var support = 'inputType' in event;
  return support;
}], ['inputeventslevel2', function (window) {
  var element = window.document.createElement('div');
  element.contentEditable = true;
  var support = 'onbeforeinput' in element;
  return support;
}]];

What would be the implications of return false instead of return support - would it make mobile editing or IMEs worse?

bobey commented 1 year ago

Hi there... just in case it helps anyone the way we fixed this was to execute this code on a Githubissues.

  • Githubissues is a development platform for aggregating issues.