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

Issue pasting certain tables #2708

Closed
juliusknorr opened this issue Jul 12, 2022 · 6 comments · Fixed by #4285
Closed

Issue pasting certain tables #2708

juliusknorr opened this issue Jul 12, 2022 · 6 comments · Fixed by #4285
Labels
1. to develop bug Something isn't working feature: formatting Features related to text formatting and node types format: markdown

Comments

@juliusknorr
Copy link
Member

juliusknorr commented Jul 12, 2022

  • Save the following table as table.html
  • Open it in the browser
  • Copy it and paste it into text
<table>
<tr style="height: 63px;">
<th style="width: 15.8024%; height: 63px;">
Country
</th>
<th style="width: 31.7902%; height: 63px;">
Partner / Service Providers
</th>
<th style="width: 26.8211%; height: 63px;">
Website
</th>
<th style="width: 10.0771%;">
Type
</th>
<th style="width: 16.8673%;">
Customer type
</th>
</tr>
<tr>
<td style="width: 15.8024%;">Australia</td>
<td style="width: 31.7902%;">Foo</td>
<td style="width: 26.8211%;"><a href="http://www.nextcloud.com.com/">http://www.nextcloud.comm/</a></td>
<td style="width: 10.0771%;">VAR</td>
<td style="width: 16.8673%;">&nbsp;</td>
</tr>
</table>

Screenshot 2022-07-12 at 08 15 46

@juliusknorr juliusknorr added the bug Something isn't working label Jul 12, 2022
@juliusknorr
Copy link
Member Author

More simplified repro:

<table>
<tr>
<th>Country</th>
<th>Partner</th>
<th>Website</th>
<th>Type</th>
<th>Customer</th>
</tr>
<tr>
<td>Australia</td>
<td>Foo</td>
<td>dfg</td>
<td>VAR</td>
<td>sdf</td>
</tr>
</table>

Screenshot 2022-07-12 at 08 26 35

@juliusknorr
Copy link
Member Author

@max-nextcloud Any idea on why that could happen?

@juliusknorr juliusknorr added feature: formatting Features related to text formatting and node types format: markdown labels Jul 12, 2022
@max-nextcloud
Copy link
Collaborator

max-nextcloud commented Jul 12, 2022

I don't get it. This looks pretty much just like our table definition.

It looks like the first within the first are not getting parsed as tableHeader. We have this rule in there:

'table > :first-child > th'

I'm pretty sure that should match. Could you try and query for that selector in the html doc?

@juliusknorr
Copy link
Member Author

Seems that browsers add a tbody to the Dom automatically:
Screenshot 2022-07-13 at 06 54 50

@max-nextcloud
Copy link
Collaborator

We should also have a rule for that:

{ 
  tag: 'table tbody :first-child th',
  priority: 60
}

The second query you tried looks more flexible and also very good. The first-child might be a tbody, thead or tr. One problem would be nested tables - as in a table nested in the first element inside another table all cells would fullfill the second query. But we don't support those anyway.

@max-nextcloud
Copy link
Collaborator

I can still reproduce this. I created a test that loads a table with that structure and that works just fine.

Maybe it's about pasting the structure.

max-nextcloud added a commit that referenced this issue Jun 12, 2023
Fixes #2708.

Try to read the first table row as headings.

Pasting works via `insertContent` while opening uses `setContent`.
They use the schema in different ways.
So we also need to make sure to test both for some corner cases.

`setContent` is fairly flexible in turning the input
into a valid document structure.
`insertContent` however fails to resolve structures
that would require picking lower priority parent elements.

Note: Some tests in src/tests/nodes/Table.spec.js
fail when using `insertContent` instead of `setContent`.
Pasting the correponding html table is fixed never the less.

Signed-off-by: Max <max@nextcloud.com>
max-nextcloud added a commit that referenced this issue Jun 12, 2023
Fixes #2708.

Try to read the first table row as headings.

Pasting works via `insertContent` while opening uses `setContent`.
They use the schema in different ways.
So we also need to make sure to test both for some corner cases.

`setContent` is fairly flexible in turning the input
into a valid document structure.
`insertContent` however fails to resolve structures
that would require picking lower priority parent elements.

Note: Some tests in src/tests/nodes/Table.spec.js
fail when using `insertContent` instead of `setContent`.
Pasting the correponding html table is fixed never the less.

Signed-off-by: Max <max@nextcloud.com>
mejo- pushed a commit that referenced this issue Jun 19, 2023
Fixes #2708.

Try to read the first table row as headings.

Pasting works via `insertContent` while opening uses `setContent`.
They use the schema in different ways.
So we also need to make sure to test both for some corner cases.

`setContent` is fairly flexible in turning the input
into a valid document structure.
`insertContent` however fails to resolve structures
that would require picking lower priority parent elements.

Note: Some tests in src/tests/nodes/Table.spec.js
fail when using `insertContent` instead of `setContent`.
Pasting the correponding html table is fixed never the less.

Signed-off-by: Max <max@nextcloud.com>
backportbot-nextcloud bot pushed a commit that referenced this issue Jun 19, 2023
Fixes #2708.

Try to read the first table row as headings.

Pasting works via `insertContent` while opening uses `setContent`.
They use the schema in different ways.
So we also need to make sure to test both for some corner cases.

`setContent` is fairly flexible in turning the input
into a valid document structure.
`insertContent` however fails to resolve structures
that would require picking lower priority parent elements.

Note: Some tests in src/tests/nodes/Table.spec.js
fail when using `insertContent` instead of `setContent`.
Pasting the correponding html table is fixed never the less.

Signed-off-by: Max <max@nextcloud.com>
backportbot-nextcloud bot pushed a commit that referenced this issue Jun 19, 2023
Fixes #2708.

Try to read the first table row as headings.

Pasting works via `insertContent` while opening uses `setContent`.
They use the schema in different ways.
So we also need to make sure to test both for some corner cases.

`setContent` is fairly flexible in turning the input
into a valid document structure.
`insertContent` however fails to resolve structures
that would require picking lower priority parent elements.

Note: Some tests in src/tests/nodes/Table.spec.js
fail when using `insertContent` instead of `setContent`.
Pasting the correponding html table is fixed never the less.

Signed-off-by: Max <max@nextcloud.com>
mejo- pushed a commit that referenced this issue Jun 20, 2023
Fixes #2708.

Try to read the first table row as headings.

Pasting works via `insertContent` while opening uses `setContent`.
They use the schema in different ways.
So we also need to make sure to test both for some corner cases.

`setContent` is fairly flexible in turning the input
into a valid document structure.
`insertContent` however fails to resolve structures
that would require picking lower priority parent elements.

Note: Some tests in src/tests/nodes/Table.spec.js
fail when using `insertContent` instead of `setContent`.
Pasting the correponding html table is fixed never the less.

Signed-off-by: Max <max@nextcloud.com>
Signed-off-by: Jonas <jonas@freesources.org>
juliusknorr pushed a commit that referenced this issue Jun 22, 2023
Fixes #2708.

Try to read the first table row as headings.

Pasting works via `insertContent` while opening uses `setContent`.
They use the schema in different ways.
So we also need to make sure to test both for some corner cases.

`setContent` is fairly flexible in turning the input
into a valid document structure.
`insertContent` however fails to resolve structures
that would require picking lower priority parent elements.

Note: Some tests in src/tests/nodes/Table.spec.js
fail when using `insertContent` instead of `setContent`.
Pasting the correponding html table is fixed never the less.

Signed-off-by: Max <max@nextcloud.com>
Signed-off-by: Jonas <jonas@freesources.org>
juliusknorr pushed a commit that referenced this issue Jun 22, 2023
Fixes #2708.

Try to read the first table row as headings.

Pasting works via `insertContent` while opening uses `setContent`.
They use the schema in different ways.
So we also need to make sure to test both for some corner cases.

`setContent` is fairly flexible in turning the input
into a valid document structure.
`insertContent` however fails to resolve structures
that would require picking lower priority parent elements.

Note: Some tests in src/tests/nodes/Table.spec.js
fail when using `insertContent` instead of `setContent`.
Pasting the correponding html table is fixed never the less.

Signed-off-by: Max <max@nextcloud.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1. to develop bug Something isn't working feature: formatting Features related to text formatting and node types format: markdown
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants