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

Table copied from Google Sheets is specified as 0px wide in the editor #13746

Closed
nedgladstone-rally opened this issue Mar 24, 2023 · 9 comments · Fixed by #13915
Closed

Table copied from Google Sheets is specified as 0px wide in the editor #13746

nedgladstone-rally opened this issue Mar 24, 2023 · 9 comments · Fixed by #13915
Assignees
Labels
squad:core Issue to be handled by the Core team. support:2 An issue reported by a commercially licensed client. type:bug This issue reports a buggy (incorrect) behavior.

Comments

@nedgladstone-rally
Copy link

nedgladstone-rally commented Mar 24, 2023

📝 Provide detailed reproduction steps (if any)

  1. Copy cells from a Google Sheets spreadsheet
  2. Paste them into a CKE5 instance that has the GeneralHtmlSupport and TableProperties plugins installed (eg, https://ckeditor.com/docs/ckeditor5/latest/examples/builds-custom/full-featured-editor.html)

✔️ Expected result

The table is visible in the editor.

❌ Actual result

The table is not visible (except for elongated borders), because it is zero width.

In the test editor above, it is zero width because the figure element wrapping the table specifies width: 0px:
figure class="table ck-widget ck-widget_with-selection-handle ck-widget_selected" style="width:0px;" contenteditable="false" data-placeholder="Type or paste your content here!"

In version 35.4.0, it is the table element itself that specifies width:0px:
table style="border-collapse:collapse;border-style:none;font-family:Arial;font-size:10pt;table-layout:fixed;width:0px;"

📃 Other details

  • Browser: Chrome
  • OS: MacOS
  • First affected CKEditor version: First version TESTED: 35.4.0
  • Installed CKEditor plugins: Removing GeneralHtmlSupport AND TableProperties plugins makes this behavior go away, but that's not an option.

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

@nedgladstone-rally nedgladstone-rally added the type:bug This issue reports a buggy (incorrect) behavior. label Mar 24, 2023
@FilipTokarski
Copy link
Member

I was able to reproduce this issue, however for me, it was not dependent on general HTML support, but on the table column resize plugin. With table column resize plugin I got result described above:

Screen.Recording.2023-03-27.at.14.57.19.mov

After removing table column resize (still with general HTML support included in the build) I got:

Screen.Recording.2023-03-27.at.14.59.15.mov

However, I got another issue with general HTML support. Just having this plugin in the build did not cause any problems. Only after adding config for this plugin, for example allow all:

htmlSupport: {
    allow: [
        {
            name: /.*/,
            attributes: true,
            classes: true,
            styles: true
        }
    ]
}

I got error and nothing was pasted:

Uncaught TypeError: Cannot read properties of null (reading 'getAttribute')
    at setAttributeOnItem (writer.ts:1810:1)
    at Writer.setAttribute (writer.ts:543:1)
    at preserveElementAttributes (table.ts:107:1)
    at dispatcher.on.priority (table.ts:91:1)
    at UpcastDispatcher.fire (emittermixin.ts:241:1)
    at UpcastDispatcher._convertItem (upcastdispatcher.ts:264:1)
    at UpcastDispatcher._convertChildren (upcastdispatcher.ts:314:1)
    at Object.convertChildren (upcastdispatcher.ts:174:1)
    at UpcastDispatcher.<anonymous> (upcasthelpers.ts:941:1)
    at UpcastDispatcher.fire (emittermixin.ts:241:1)

I checked everything on latest master. @nedgladstone-rally could you check and confirm that those findings apply also for your editor?

@martynawierzbicka martynawierzbicka added the support:2 An issue reported by a commercially licensed client. label Mar 27, 2023
@nedgladstone-rally
Copy link
Author

nedgladstone-rally commented Mar 27, 2023

@FilipTokarski Thank you for looking!

Yes, I get the same "TypeError: Cannot read properties of null (reading 'getAttribute')" when allowing name: /.*/, in htmlSupport.
If I set attributes: true there, then general chaos breaks out.

We have TableColumnResize temporarily removed due to #13492 so I'm seeing the zero-width pastes without that plugin.

In our environment, if I remove BOTH the GeneralHtmlSupport AND TableProperties plugins, I'm able to paste from Sheets, albeit getting a table that ignores the Sheets column widths and fills the editor window.
image

I'll correct above where I called out just GeneralHtmlSupport, because disabling ONLY GeneralHtmlSupport and leaving TableProperties enabled does NOT allow pasting for me, either.

Our htmlSupport config is:

    htmlSupport: {
      allow: [
        {
          name: /^(table|thead|tbody|td|th|tr|col|colgroup)$/,
          classes: true,
          styles: true,
        },
      ],
    },

These are the plugins we have enabled:
Autoformat,
AutoLink,
Bold,
Code,
CodeBlock,
DocumentList,
Essentials,
Font,
GeneralHtmlSupport,
Image,
ImageInsert,
ImageResize,
ImageTextAlternative,
ImageToolbar,
Indent,
IndentBlock,
Italic,
Link,
Mention,
MentionsToolbar,
Paragraph,
PasteFromOffice,
RemoveFormat,
Strikethrough,
Table,
TableCellProperties,
TableProperties,
TableToolbar,
Underline

@Witoso Witoso added the squad:core Issue to be handled by the Core team. label Mar 28, 2023
@Reinmar
Copy link
Member

Reinmar commented Mar 28, 2023

Quick tip (without reading the thread): We changed a lot in how tables, colgroup/col, GHS (see #11479), and column resize work together. But since the alpha releases (of the OSS part of the project) are out already you can actually test those changes.

@nedgladstone-rally
Copy link
Author

@Reinmar I tested with 37.0.0-alpha.3 (displays 37.0.0-alpha.2 for CKEDITOR_VERSION), and see the same misbehavior described above.

@FilipTokarski
Copy link
Member

FilipTokarski commented Apr 3, 2023

I checked it on 36.0.1 and I was able to reproduce it. This issue (table pasted with zero width) is dependent on a few factors and has different outcomes based on those factors:

  • if there is table column resize in the editor, it will happen regardless of other plugins and configurations, there will be no error in the console
  • if there is no table column resize in the editor, it will happen when GHS allows for styles on table elements, there will be no error in the console
  • if GHS config allows for all, pasting will result in error
  • if there is no GHS (or no GHS config for table) and no table column resize, it will paste properly

Steps to reproduce in manual tests:

  1. Go to ghs-all-features.html manual test.
  2. Remove table column resize plugin.
  3. Set the following GHS config:
    htmlSupport: {
      allow: [
        {
          name: /^(table|thead|tbody|td|th|tr|col|colgroup)$/,
          styles: true,
        },
      ],
    },
  1. Paste a table from Google Sheets into the editor

Result:

Screenshot 2023-04-03 at 15 20 43

When you then comment out styles: true from GHS config and try again, you will get proper result:

Screenshot 2023-04-03 at 15 24 20

Error in the console seems like a separate issue here. It happens when GHS config allows for everything - described in one of my previous comments.

@Witoso
Copy link
Member

Witoso commented Apr 5, 2023

The issue possibly is caused by width: 0px; that is coming directly from the Google Sheets side in the clipboard (very strange markup):

  <table
    xmlns="http://www.w3.org/1999/xhtml"
    cellspacing="0"
    cellpadding="0"
    dir="ltr"
    border="1"
    style="
      table-layout: fixed;
      font-size: 10pt;
      font-family: Arial;
      width: 0px; 
      border-collapse: collapse;
      border: none;
    "
  >

Scope:

@CKEditorBot CKEditorBot added the status:planned Set automatically when an issue lands in the "Sprint backlog" column. We will be working on it soon. label Apr 5, 2023
@Witoso
Copy link
Member

Witoso commented Apr 13, 2023

Decided to split the issue. GHS crash will be done in this issue: #13876

@filipsobol filipsobol self-assigned this Apr 17, 2023
@CKEditorBot CKEditorBot added status:in-progress Set automatically when an issue lands in the "In progress" column. We are working on it. status:planned Set automatically when an issue lands in the "Sprint backlog" column. We will be working on it soon. and removed status:planned Set automatically when an issue lands in the "Sprint backlog" column. We will be working on it soon. status:in-progress Set automatically when an issue lands in the "In progress" column. We are working on it. labels Apr 17, 2023
arkflpc added a commit that referenced this issue Apr 21, 2023
…s-support

Fix (html-support): Fix editor crashes when pasting table from Google Sheets. Closes #13876.

Fix (paste-from-office): Fix width of tables pasted from Google Sheets. Closes #13746.
@CKEditorBot CKEditorBot removed the status:in-progress Set automatically when an issue lands in the "In progress" column. We are working on it. label Apr 21, 2023
@CKEditorBot CKEditorBot added this to the iteration 63 milestone Apr 21, 2023
@nedgladstone-rally
Copy link
Author

Woohoo!

@Witoso
Copy link
Member

Witoso commented Apr 24, 2023

Yes, this is gonna be a part of the next release 😊

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

Successfully merging a pull request may close this issue.

7 participants