ckeditor / ckeditor4

The best enterprise-grade WYSIWYG editor. Fully customizable with countless features and plugins.
https://ckeditor.com/ckeditor-4
Other
5.8k stars 2.48k forks source link

Style property is undefined #4204

Open sergeymosyakovroko opened 4 years ago

sergeymosyakovroko commented 4 years ago

https://github.com/ckeditor/ckeditor4/blob/965f621e6ab15f7f62345b9b21785d9fdf02895d/plugins/pastefromgdocs/filter/default.js#L144

We use workaround proposed in https://github.com/ckeditor/ckeditor4/issues/469. However sometimes code does not work due to specific issue where style attribute is not exists because createStyleStack function removes it.

Can we additional check that style attribute is exists please?

Thanks

Reproduction steps

  1. Force paste dialog (e.g. using code proposed in https://github.com/ckeditor/ckeditor4/issues/4204#issuecomment-675356900 or by using/emulating touch device).
  2. Copy some styled content from Google Docs.
  3. Press "Paste as plain text" button.
  4. Paste content into the dialog and press "Ok".

Expected result

Content is pasted into the editor.

Actual result

Error is thrown in the console:

default.js:156 Uncaught TypeError: Cannot read property 'replace' of undefined
    at handleSuperAndSubScripts (default.js:156)
    at span (default.js:71)
    at filterRulesGroup.exec (filter.js:335)
    at $.onElement (filter.js:201)
    at CKEDITOR.htmlParser.element.filter (element.js:180)
    at CKEDITOR.htmlParser.element.filterChildren (fragment.js:554)
    at CKEDITOR.htmlParser.element.filter (element.js:247)
    at CKEDITOR.htmlParser.fragment.filterChildren (fragment.js:554)
    at CKEDITOR.htmlParser.fragment.filter (fragment.js:519)
    at $.applyTo (filter.js:168)
jacekbogdanski commented 4 years ago

Hello @sergeymosyakovroko, I'm not sure which workaround you have in mind (there are a couple of code snippets there). Nevertheless, this workaround has been proposed by the community, so I suppose you may have better luck just by asking in #469.

Please note that our GitHub issue tracker serves for reporting bugs and new features only. Unfortunately, your request is neither of them, therefore please you should leave your question on Stack Overflow where our team and the community assist other users in solving their issues. Also, since CKEditor is an Open Source software, we believe it’s crucial to share proposed solutions and make others benefit from them.

If you have CKEditor license, please contact our support team. You also can ask your implementation questions at Stack Overflow.

sergeymosyakovroko commented 4 years ago

@jacekbogdanski If you think that script error is not a bug then something wrong there. Yes we have license for your product but I am not planning to spend time to ask to fix something there because it is much easier to fix issue in package we use. I thought CKEDITOR team worries about the quality of the product. However it sounds like it is not true. In the following function if style attribute is not presented code will fail. One of the copy paste plugins removes this attribute due to unknown reason to me.

Thank you for your support!!!

function handleSuperAndSubScripts( element ) { var superScriptRegex = /vertical-align:\ssuper/, subScriptRegex = /vertical-align:\ssub/, replaceRegex = /vertical-align\s*.+?;?/, style = element.attributes.style;

    if ( superScriptRegex.test( style ) ) {
        element.name = 'sup';
    } else if ( subScriptRegex.test( style ) ) {
        element.name = 'sub';
    }

    element.attributes.style = style.replace( replaceRegex, '' );
}
jacekbogdanski commented 4 years ago

The reason why we are not changing CKEditor 4 code for every request is that we actually care about the quality of the product and would like to keep it regression free for every user.

Please, if you are able to provide some reproduction steps or code which would allow us to investigate the issue and make sure that's something which is indeed a CKEditor 4 bug, we would be very glad if you could share it with us.

Yes we have license for your product but I am not planning to spend time to ask to fix something there because it is much easier to fix issue in package we use.

I really encourage you to contact the support team if you have a license, that's what it's for.

sergeymosyakovroko commented 4 years ago

To reproduce issue I mentioned you need to use snippet code proposed in https://github.com/ckeditor/ckeditor4/issues/469:

CKEDITOR.on("instanceReady", function(event) { event.editor.on("beforeCommandExec", function(event) { // Show the paste dialog for the paste buttons and right-click paste if (event.data.name == "paste") { event.editor._.forcePasteDialog = true; } // Don't show the paste dialog for Ctrl+Shift+V if (event.data.name == "pastetext" && event.data.commandData.from == "keystrokeHandler") { event.cancel(); } }) });

Then you need to use button "Paste as plain text". Copy paste text from google docs into opened dialog. You need to be sure pasted object contains "span" element. Then you need to press OK button and script error will happen. We are able to reproduce it in Chrome (latest) in Windows and Mac. As I mentioned it will happen because plugin code will remove style attribute from attributes object and when code will try to use it without check it will fail. I do not see any reason to contact support because we already fixed issue in our version. We also mentioned it to you so you can fix it going forward. We tried to help because we think that CKEDITOR is one of the best products we used.

jacekbogdanski commented 4 years ago

Thank you @sergeymosyakovroko for reproduction steps. @Comandeer since you have been mostly involved in creating this feature, could you take a look at the issue?

Comandeer commented 4 years ago

I can confirm the issue. It's also an issue on mobile devices, where we still use Paste Dialog to handle pasting. I'll update the initial post with reproduction steps.

aviral2311 commented 4 years ago

@Comandeer I am using v4.14.1 and when I try to use Cut Copy Paste buttons, cut and copy work, but Paste gives me error -

ckeditor.js?ts=1601170744000:11224 Uncaught TypeError: Cannot read property 'replace' of undefined
    at CKEDITOR.command.exec (ckeditor.js?ts=1601170744000:11224)
    at CKEDITOR.command.exec (ckeditor.js?ts=1601170744000:3455)
    at a.execCommand (ckeditor.js?ts=1601170744000:4720)
    at CKEDITOR.ui.button.CKEDITOR.tools.extend.click (ckeditor.js?ts=1601170744000:10836)
    at Object.execute (ckeditor.js?ts=1601170744000:10854)
    at ckeditor.js?ts=1601170744000:10865
    at ckeditor.js?ts=1601170744000:456
    at Object.callFunction (ckeditor.js?ts=1601170744000:458)
    at HTMLAnchorElement.onclick (view.object.do?ctxtId=89&viewId=567&ctxt=Policy&submitAction=link&prmId=12076&ctxId=12076&prmtId=89&prmt=Policy:1)

This is basically due to this line. I have installed full version of ckeditor. f = "string" === typeof e ? e : a.lang.clipboard.pasteNotification.replace(/%1/, '\x3ckbd aria-label\x3d"' + m.aria + '"\x3e' + m.display + "\x3c/kbd\x3e"),

I dont know if this issue is the right one to track.

nehajyoti04 commented 3 years ago

To reproduce issue I mentioned you need to use snippet code proposed in #469:

CKEDITOR.on("instanceReady", function(event) { event.editor.on("beforeCommandExec", function(event) { // Show the paste dialog for the paste buttons and right-click paste if (event.data.name == "paste") { event.editor._.forcePasteDialog = true; } // Don't show the paste dialog for Ctrl+Shift+V if (event.data.name == "pastetext" && event.data.commandData.from == "keystrokeHandler") { event.cancel(); } }) });

Then you need to use button "Paste as plain text". Copy paste text from google docs into opened dialog. You need to be sure pasted object contains "span" element. Then you need to press OK button and script error will happen. We are able to reproduce it in Chrome (latest) in Windows and Mac. As I mentioned it will happen because plugin code will remove style attribute from attributes object and when code will try to use it without check it will fail. I do not see any reason to contact support because we already fixed issue in our version. We also mentioned it to you so you can fix it going forward. We tried to help because we think that CKEDITOR is one of the best products we used.

I too am facing exactly same issue. Please HELP!

arunp-keypress commented 3 years ago

bvb

johnhyde commented 3 years ago

I encountered this issue when copying text which came from a Google Doc and pasting it in the editor with CMD+V. I found a workaround which doesn't involve forking ckeditor4. Maybe someone else will find this useful. I run this code right after loading ckeditor4.

// This is a workaround for a bug in pastefromgdocs where it tries
// to replace the style attribute without checking that it's there
// https://github.com/ckeditor/ckeditor4/blob/05e2b1115beb0885a637adb81e7ed091b47a2d8c/plugins/pastefromgdocs/filter/default.js#L156
// To fix this, we wrap createStyleStack, which it calls, and make
// sure that the style attribute is present, even if it's empty...
// Unfortunately this function isn't available until the filters
// are loaded, so we also wrap some other functions to make sure
// we can wrap createStyleStack in time.

(function() {
  let pastetools = window.CKEDITOR.plugins.pastetools;
  function tryWrapCreateStyleStack() {
    let createStyleStack = pastetools.filters.common?.styles?.createStyleStack;
    if (createStyleStack && !createStyleStack.wrapped) {
      pastetools.filters.common.styles.createStyleStack = (element, ...args) => {
        let retVal = createStyleStack(element, ...args);
        element.attributes.style = element.attributes.style || '';
        return retVal;
      };
      pastetools.filters.common.styles.createStyleStack.wrapped = true;
    }
  }
  let createFilter = pastetools.createFilter;
  pastetools.createFilter = function (...args) {
    let retVal = createFilter.call(this, ...args);
    tryWrapCreateStyleStack();
    return retVal;
  };
  let loadFilters = pastetools.loadFilters;
  pastetools.loadFilters = function (...args) {
    let retVal = loadFilters.call(this, ...args);
    tryWrapCreateStyleStack();
    return retVal;
  };
})();