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

Froala code view editor - image CSS getting stripped #5620

Closed

Conversation

GaberNeighbor
Copy link
Contributor

Q A
Bug fix? x
New feature?
Automated tests included?
Related user documentation PR URL
Related developer documentation PR URL
Issues addressed (#s or URLs)
BC breaks?
Deprecations?

Description:

Removing call to function in Froala image.js that removes many common style declarations and replaces them with a broken class.

Steps to reproduce the bug:

  1. Create new email
  2. Open builder
  3. Insert image in text-slot
  4. Open code view
  5. Give image inline CSS: style="margin: 0; float: right; display: block; z-index: 1; position: relative; overflow: auto; vertical-align: top"
  6. Close code view, then re-open
  7. Observe that all inline CSS has been stripped from the image (very bad for emails).

Steps to test this PR:

  1. Repeat steps 1-6 above
  2. Observe that all inline CSS is no longer removed from the image

@escopecz
Copy link
Sponsor Member

It's never a good idea to hack a library. Because once that library gets updated it will remove the hack. We use an older version of Froala. Maybe there is fix for this issue already in the new version. Or there is hopefully some configuration option on how to avoid it. Could you please look at another option?

@kuzmany
Copy link
Member

kuzmany commented Jan 22, 2018

What about #5174 ? Works for you?
Don't forget run php app/console m:a:g after apply PR to regenerate assets.

@escopecz escopecz added bug Issues or PR's relating to bugs pending-feedback PR's and issues that are awaiting feedback from the author labels Jan 23, 2018
@GaberNeighbor
Copy link
Contributor Author

@kuzmany #5174 is a better solution than mine. @escopecz does #5174 work for you?

@escopecz
Copy link
Sponsor Member

I'll close this PR. Let's discuss this issue at #5174 too since the solution will be very similar.

@escopecz escopecz closed this Jan 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issues or PR's relating to bugs pending-feedback PR's and issues that are awaiting feedback from the author
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants