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

Content filtering – tests, fixes and stability #341

Closed
Reinmar opened this issue Sep 29, 2016 · 7 comments
Closed

Content filtering – tests, fixes and stability #341

Reinmar opened this issue Sep 29, 2016 · 7 comments
Labels
type:bug This issue reports a buggy (incorrect) behavior. type:feature This issue reports a feature request (an idea for a new functionality or a missing option). type:improvement This issue reports a possible enhancement of an existing feature. type:task This issue reports a chore (non-production change) and other types of "todos".
Milestone

Comments

@Reinmar
Copy link
Member

Reinmar commented Sep 29, 2016

Currently, it's possible to load e.g.:

<ul><li><h2>dfdd</h2></li></ul>

and you're even able to retrieve this data back :D.

The schema doesn't allow headings in list items:

schema.registerItem( 'listItem', '$block' );
schema.allow( {
    name: 'listItem',
    inside: '$root',
    attributes: [ 'type', 'indent' ]
} );

So the heading or list should be filtered out. There will be many cases like this. Elements that we don't support or which combinations we don't support should be somehow, gracefully, handled upon conversion to the model.

This requires a lot of tests and most likely many fixes. So far we haven't tested for that and haven't used schema extensively. There will be time when we'll need to do that and this will be we'll want to filter pasted content in a stable way.

@Reinmar Reinmar added type:bug This issue reports a buggy (incorrect) behavior. status:confirmed type:task This issue reports a chore (non-production change) and other types of "todos". type:improvement This issue reports a possible enhancement of an existing feature. type:feature This issue reports a feature request (an idea for a new functionality or a missing option). labels Sep 29, 2016
@Reinmar Reinmar changed the title Content filtering – tests, fixes and stability. Content filtering – tests, fixes and stability Sep 29, 2016
@scofalik
Copy link
Contributor

That's true, there is no check in list item view-to-model converter and this is mistake. However, default converters have schema checks. There is schema filtering on view-to-model conversion, in buildViewConverter.

So I agree this is more overall issue, that we don't care about schema, but this could be also reported as a bug in List feature.

@Reinmar
Copy link
Member Author

Reinmar commented Oct 13, 2016

Important use case: #348. It's also needed for pasting.

@Reinmar Reinmar added this to the iteration 5 milestone Oct 28, 2016
@Reinmar Reinmar modified the milestones: iteration 6, iteration 5 Nov 28, 2016
@Reinmar Reinmar modified the milestones: iteration 8, iteration 6 Feb 2, 2017
@Reinmar Reinmar modified the milestones: iteration 9, iteration 8 Mar 6, 2017
@Reinmar Reinmar modified the milestones: iteration 10, iteration 9 Apr 5, 2017
@Reinmar
Copy link
Member Author

Reinmar commented May 3, 2017

We closed something around 20 tickets in ckeditor5-paragraph, ckeditor5-image and ckeditor5-lists improving the stability of pasting and general conversion's UX (e.g. aligning disallowed structures to the editor's schema). I can still crash the editor from time to time but I think that we're on a level of stability where we can say that pasting is well covered.

@Reinmar Reinmar closed this as completed May 3, 2017
@scofalik
Copy link
Contributor

scofalik commented May 4, 2017

Could you point me to examples which crash editor (or is it just "copy a website that is big and rich enough")? Any idea what is crashing?

@Reinmar
Copy link
Member Author

Reinmar commented May 4, 2017

E.g. https://www.theatlantic.com/entertainment/archive/2017/04/the-why-of-cooking-samin-nosrat/523923/ (if you copy the entire content)

Previously I've some other website crashing but with the same error, I think.

@scofalik
Copy link
Contributor

scofalik commented May 4, 2017

Ah, goddamn those bugs in Mapper :).

Anyway it's worth looking at somewhere in near future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug This issue reports a buggy (incorrect) behavior. type:feature This issue reports a feature request (an idea for a new functionality or a missing option). type:improvement This issue reports a possible enhancement of an existing feature. type:task This issue reports a chore (non-production change) and other types of "todos".
Projects
None yet
Development

No branches or pull requests

2 participants