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

Bug: Incomplete CSS margin parsing if "auto" or "0" without unit is used #1330

Closed
sserdyuk opened this issue Dec 6, 2017 · 7 comments
Closed
Assignees
Labels
core The issue is caused by the editor core code. good first issue Relatively easy to fix. This is a perfect issue if you are willing to create a Pull Request. status:confirmed An issue confirmed by the development team. type:bug A bug.
Milestone

Comments

@sserdyuk
Copy link

sserdyuk commented Dec 6, 2017

Are you reporting a feature request or a bug?

Bug

Related to #450

Open HTML with inline CSS like margin: 10px 0 0; switch to Source mode, or
open HTML with inline CSS like margin: 10px auto 0; switch to Source mode.

Expected result

margin-bottom:0; margin-left:0; margin-right:0; margin-top:10px, or
margin-bottom:0; margin-left:auto; margin-right:auto; margin-top:10px

Actual result

margin-bottom:10px; margin-left:10px; margin-right:10px; margin-top:10px

What is the actual result of the above steps?

Incorrect margins are applied

@msamsel msamsel added status:confirmed An issue confirmed by the development team. type:bug A bug. labels Dec 18, 2017
@engineering-this engineering-this added the plugin:indent The plugin which probably causes the issue. label Jun 15, 2018
@engineering-this
Copy link
Contributor

engineering-this commented Jun 15, 2018

I just found that problem when working with PFW.

There are more cases of buggy margins:

Case 1:

//Input:
<p style="margin: 0 0 12px 0">ASD</p>
//Expected:
<p style="margin-bottom: 12px">ASD</p>
//Actual:
<p style="margin-left:12px; margin-right:12px">ASD</p>

Case 2:

//Input:
<p style="margin: 0 12px 6px">ASD</p>
//Expected:
<p style="margin-left: 12px; margin-bottom: 6px; margin-right: 12px">ASD</p>
//Actual:
<p style="margin-left:6px; margin-right:6px">ASD</p>

And probably many other cases.

@achrysanthakopoulou
Copy link

Is this bug going to be fixed ?
It seems that I didn't have this issue until I used this code in my config:

config.allowedContent = {
    $1: {
        elements: CKEDITOR.dtd,
        attributes: true,
            styles: true,
        classes: true
    }
};

@achrysanthakopoulou
Copy link

Anyone?

@f1ames f1ames added this to the Backlog milestone Aug 21, 2019
@f1ames
Copy link
Contributor

f1ames commented Aug 21, 2019

Hello @achrysanthakopoulou,
I have added this issue to our Backlog, however as we have other priorities at the moment, there is no ETA for this one.

For anyone interested in fixing this issue, please add 👍 reaction to the issue report.

@achrysanthakopoulou
Copy link

Hello @f1ames
I tried with a colleague of mine to fix the issue.

Please check the following code:
Replace this regex at production's ckeditor.js main file:
.match(/(-?[.\d]+\w+)/g)
with this:
.match(/(-?[.\d]*\w+)/g)

It solves my problem but needs testing.

At ckeditor-dev you can find it in :
https://github.com/ckeditor/ckeditor-dev/blob/606ced7/core/filter.js#L2201

@f1ames
Copy link
Contributor

f1ames commented Aug 21, 2019

@achrysanthakopoulou thanks for checking this issue!

Maybe you would like to prepare a PR with the fix (please see contributing guide)?

@f1ames f1ames added good first issue Relatively easy to fix. This is a perfect issue if you are willing to create a Pull Request. core The issue is caused by the editor core code. and removed plugin:indent The plugin which probably causes the issue. labels Aug 21, 2019
@hub33k hub33k self-assigned this Sep 10, 2020
@f1ames
Copy link
Contributor

f1ames commented Oct 2, 2020

Fixed in #4270.

@f1ames f1ames closed this as completed Oct 2, 2020
@f1ames f1ames modified the milestones: Backlog, 4.15.1 Oct 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core The issue is caused by the editor core code. good first issue Relatively easy to fix. This is a perfect issue if you are willing to create a Pull Request. status:confirmed An issue confirmed by the development team. type:bug A bug.
Projects
None yet
Development

No branches or pull requests

6 participants