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.

Sign up for GitHub

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

Jump to bottom

No margin determined from the shortcut syntax #450

Closed
mlewand opened this issue Jun 1, 2017 · 1 comment
Closed

No margin determined from the shortcut syntax #450

mlewand opened this issue Jun 1, 2017 · 1 comment
Labels
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. support An issue reported by a commercially licensed client. type:bug A bug.
Milestone
4.7.1

Comments

@mlewand
Copy link
Contributor

mlewand commented Jun 1, 2017

Are you reporting a feature or a bug?

Bug

Provide detailed reproduction steps (if any)

  1. Open CKEditor sample page.
  2. Use "Source" button to set the content to:
<p style="margin:30px;">I&#39;m an instance of <a href="http://ckeditor.com">CKEditor</a>.</p>
  1. Go back to WYSIWYG mode by clicking "Source" button again.

Expected result

Paragraph retains margin-left from given margin property.

Note that margin as well as margin-right, margin-top, margin-bottom properties should get filtered out, as they are not allowed by ACF in default config.

Actual result

Margin is filtered out completely.
@mlewand mlewand added status:confirmed An issue confirmed by the development team. type:bug A bug. labels Jun 1, 2017
@mlewand
Copy link
Contributor Author

mlewand commented Jun 1, 2017

It's caused by a buggy transformation in case when only one number is given to shortcut format - https://github.com/ckeditor/ckeditor-dev/blob/606ced7/core/filter.js#L2204.

@mlewand mlewand self-assigned this Jun 1, 2017
@mlewand mlewand changed the title No margin determined from the shorcut syntax No margin determined from the shortcut syntax Jun 1, 2017
@mlewand mlewand added support An issue reported by a commercially licensed client. good first issue Relatively easy to fix. This is a perfect issue if you are willing to create a Pull Request. labels Jun 1, 2017
@mlewand mlewand added this to the Backlog milestone Jun 2, 2017
@mlewand mlewand mentioned this issue Jun 2, 2017
Merged
@mlewand mlewand removed their assignment Jun 2, 2017
Comandeer added a commit that referenced this issue Jun 2, 2017
@Comandeer
Merge pull request #458 from ckeditor/t/450
12f0eff 
Fixed splitMarginShorthand transformation

Fixes #450.
@mlewand mlewand modified the milestones: 4.7.1, Backlog Jun 2, 2017
@sserdyuk sserdyuk mentioned this issue Dec 6, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
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. support An issue reported by a commercially licensed client. type:bug A bug.
Projects
None yet
Milestone
4.7.1
Development

No branches or pull requests
1 participant
@mlewand