CK Editor adds in newlines which are rendered as empty paragraph tags.

[robertmassaioli]

I think that I am running into this exception and then the black lines are being wrapped by paragraph tags in the CK Editor: http://ckeditor.com/comment/62946#comment-62946

I am not sure what the solution is yet. More investigation forthcoming.

[stevensouthard]

I'm looking forward to the result of your investigation.

[robertmassaioli]

I found the source of the problem. It is this function in spree itself that wraps every render of the product description:

# converts line breaks in product description into <p> tags (for html display purposes)
def product_description(product)
  raw(product.description.gsub(/(.*?)\r?\n\r?\n/m, '<p>\1</p>'))
end

It runs on every single render of the product page:

<div itemprop="description" data-hook="description">
  <%= product_description(@product) rescue t(:product_has_no_description) %>
</div>

This is not cool and I am going to come up with a way of making sure that spree editor can mandate that spree never runs that function. We can safely assume that our editors will generate the correct HTML for the page and they should most certainly not need to pass through a regex every single time that a user requests for it to be rendered.

@radar Do you have any particular way that you would like me to implement this fix? Otherwise I am just going to give it a show and try and make it configurable via something in config.

[robertmassaioli]

Just to show you what is happening: ckeditor generates the following HTML:

<p>hello world</p>

<p>tihs is completely awesome and it works</p>

<p>why so many spaces in the code. and why some more formatting afterwards?</p>

And spree, via the product_description function, converts it to the following:

<p><p>hello world</p></p><p><p>tihs is completely awesome and it works</p></p><p>why so many spaces in the code. and why some more formatting afterwards?</p>\n

So yes, need to find a solution to this problem before I can use this editor live.

[robertmassaioli]

@stevensouthard Initial investigation complete I guess. Solution pending. I am trying some things now.

[stevensouthard]

I'm so happy your on this. CK editor is a great addition to spree and
I can't see delivering my spree project without it. I found this
issue a couple of weeks ago and I thought the problem was with me or
CK editor. I spent a couple of hours working on it even yesterday
before I found your issue post. Thank you.

Steven

On Apr 10, 2013, at 9:49 AM, Robert Massaioli wrote:

@stevensouthard Initial investigation complete I guess. Solution
pending. I am trying some things now.

—
Reply to this email directly or view it on GitHub.

[robertmassaioli]

@stevensouthard Well, I have fixed the bug locally. You can check out these branches:

• Spree: https://github.com/robertmassaioli/spree/tree/optional-product-description-formatting
• Spree Editor: https://github.com/robertmassaioli/spree_editor/tree/raw-product-description

But now I have to get those changes merged in to spree and that involves writing test cases and stuff: https://github.com/robertmassaioli/spree/blob/master/CONTRIBUTING.md

Since it is 1:14am in the morning here I am going to bed. It is unlikely that I will have too much time for this but an help on my changes to get it ready for a proper PR against spree would be much appreciated.

[stevensouthard]

Thank you for getting that going. I took a look at it and I might
have to do some updating to get that working. I'm on 1.2.4. I can't
decide if I should update to 1.3.3 or incorporate your changes into
1.2.4. I'd love to give you a hand with a proper PR against spree but
I don't really know how to do that. I'll be keeping an eye on your
progress and I am very thankful for your efforts.

Steven

On Apr 10, 2013, at 10:14 AM, Robert Massaioli wrote:

@stevensouthard Well, I have fixed the bug locally. You can check
out these branches:

Spree: https://github.com/robertmassaioli/spree/tree/optional-product-description-formatting
Spree Editor: https://github.com/robertmassaioli/spree_editor/tree/raw-product-description
But now I have to get those changes merged in to spree and that
involves writing test cases and stuff: https://github.com/robertmassaioli/spree/blob/master/CONTRIBUTING.md

Since it is 1:14am in the morning here I am going to bed. It is
unlikely that I will have too much time for this but an help on my
changes to get it ready for a proper PR against spree would be much
appreciated.

—
Reply to this email directly or view it on GitHub.

[robertmassaioli]

@stevensouthard There is no reason that you could not backport this change. What you should do is jump on the Spree Core PR and let them know that you would really like them to pull it back into previous versions of spree. But for my purposes I am living close to master.

And no problems, any help you could give at all, even to say that this affects you too would help get traction on these issues. Thanks for the support!

[stevensouthard]

So I made the changes to my fork of spree 1.2.4 and so now do I need
to add an initializer or something to make it work?

On Apr 11, 2013, at 9:48 AM, Robert Massaioli wrote:

@stevensouthard There is no reason that you could not backport this
change. What you should do is jump on the Spree Core PR and let them
know that you would really like them to pull it back into previous
versions of spree. But for my purposes I am living close to master.

And no problems, any help you could give at all, even to say that
this affects you too would help get traction on these issues. Thanks
for the support!

—
Reply to this email directly or view it on GitHub.

[JDutil]

@robertmassaioli I'm closing this as it should be fixed by your PR