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

Chrome 105 breaks slate 0.27.x #5110

Open
beaugunderson opened this issue Sep 1, 2022 · 36 comments · May be fixed by semji/slate#1
Open

Chrome 105 breaks slate 0.27.x #5110

beaugunderson opened this issue Sep 1, 2022 · 36 comments · May be fixed by semji/slate#1
Labels

Comments

@beaugunderson
Copy link

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

  • Slate Version: 0.27.4
  • Operating System: macOS
  • Browser: Chrome 105
@beaugunderson
Copy link
Author

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
Copy link
Author

beaugunderson commented Sep 1, 2022

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
Copy link
Author

beaugunderson commented Sep 1, 2022

Chrome 104:

image

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

image

@beaugunderson
Copy link
Author

Realized I never included the traceback above:
image

@zarv1k
Copy link
Contributor

zarv1k commented Sep 1, 2022

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
Copy link
Contributor

zarv1k commented Sep 2, 2022

@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
Copy link

eMerzh commented Sep 2, 2022

it's related to #5108 no?

@zarv1k
Copy link
Contributor

zarv1k commented Sep 2, 2022

it's related to #5108 no?

#5108 (comment)

@fikrikarim
Copy link

fikrikarim commented Sep 2, 2022

@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
Copy link
Contributor

zarv1k commented Sep 2, 2022

@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 dylans changed the title Chrome 105 breaks slate Chrome 105 breaks slate 0.27.x Sep 2, 2022
@dylans
Copy link
Collaborator

dylans commented Sep 2, 2022

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
Copy link

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

@dylans
Copy link
Collaborator

dylans commented Sep 2, 2022

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
Copy link
Author

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
Copy link

Taelar commented Sep 5, 2022

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
Copy link
Contributor

zarv1k commented Sep 5, 2022

@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.

@Taelar
Copy link

Taelar commented Sep 5, 2022

@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
Copy link

piciuok commented Sep 5, 2022

We have same problem with version:

  • slate 0.44.22
  • slate-react 0.21.23

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
Copy link

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
Copy link

Taelar commented Sep 6, 2022

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
Copy link

roman-korovets commented Sep 6, 2022

@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
Copy link
Author

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
Copy link

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
Copy link

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

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

  "resolutions": {
    "slate-dev-environment": "0.1.4"
  },

@Nantris
Copy link
Contributor

Nantris commented Sep 7, 2022

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?

lennym added a commit to UKHomeOffice/asl-projects that referenced this issue Sep 16, 2022
Override the `-webkit-user-modify: read-write-plaintext-only` style that is added by slate to the editor element that causes a cursor jumping bug in Chrome 105.

Details at ianstormtaylor/slate#5110
@dennisrcao
Copy link

@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

  • cursor jumps to the end of a line when I attempt to insert text in the middle of the line
  • bold, italics, subscript and superscript aren't working with the textbox
  • placeholder text (greyed out) became real text when you clicked on it.

But now they are fixed, and I used the following modification:
...(readOnly || (!IS_IOS && !IS_SAFARI) ? {} : { WebkitUserModify: 'read-write-plaintext-only' }),
(line 600 of slate-react/src/components/content.js)

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

Hope that helps someone.

@stowball
Copy link

stowball commented Oct 5, 2022

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
Copy link

bobey commented Oct 6, 2022

@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
Copy link
Contributor

Nantris commented Oct 6, 2022

@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
Copy link

bobey commented Oct 7, 2022

@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
Copy link
Contributor

Nantris commented Oct 8, 2022

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
Copy link

mathisobadia commented Oct 13, 2022

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;
 
-    var isSynthetic = !!event.nativeEvent;
+    // var isSynthetic = !!event.nativeEvent;
+    var isSynthetic = true;
 
     // If the event is synthetic, it's React's polyfill of `beforeinput` that
     // isn't a true `beforeinput` event with meaningful information. It only
diff --git a/node_modules/slate-react/lib/slate-react.js b/node_modules/slate-react/lib/slate-react.js
index 6640565..b82eb19 100644
--- a/node_modules/slate-react/lib/slate-react.js
+++ b/node_modules/slate-react/lib/slate-react.js
@@ -4027,8 +4027,8 @@ function AfterPlugin() {
   function onBeforeInput(event, editor, next) {
     var value = editor.value;
 
-    var isSynthetic = !!event.nativeEvent;
-
+    //var isSynthetic = !!event.nativeEvent;
+    var isSynthetic = true;
     // If the event is synthetic, it's React's polyfill of `beforeinput` that
     // isn't a true `beforeinput` event with meaningful information. It only
     // gets triggered for character insertions, so we can just insert directly.

run yarn / npm install and that's it

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
Copy link

bobey commented Oct 19, 2022

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
Copy link

nbarrera commented Oct 31, 2022

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 🤷

@Nantris
Copy link
Contributor

Nantris commented Nov 1, 2022

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
Copy link

bobey commented Nov 3, 2022

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

Thank you so much @nbarrera, your tips helped us temporarily fix our issue with ê, â ... characters.

bengotow added a commit to Foundry376/Mailspring that referenced this issue Nov 20, 2023
Was able to determine that this was the problem by bisecting our Electron upgrade and identifying which chrome version bump broke it, and then googling for slate + chrome version.

It looks like this entire concept of webkit-user-modify is deprecated, so I have no idea why this matters, but it looks like they changed the default to read-write-plaintext and it broke Slate for everyone.

ianstormtaylor/slate#5110
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet