Use <erb> instead of <code> when parsing templates #105
Conversation
This changes the ERB template parser to use `<erb>` rather than `<code>`
to designate code blocks in a template.
For example:
<%= render template: 'foo/bar' %>
Used to parse as:
<code erb-loud> render template: 'foo/bar' </code>
But not parses as:
<erb erb-loud> render template: 'foo/bar' </erb>
This fixes a number of problems that result from using `<code>`:
* There may be </code> tags in the original ERB, which would be turned
into `%>`
(Fixes spree#101)
* Any <code> tags in the <head> of a document would be moved to the
body, since <code> is a real HTML tag, and isn't allowed in the head.
This happens in nokogiri under jRuby or libXML 2.9.0
(Fixes spree#84, Fixes spree#100)
Unfortunately this breaks any existing deface selectors looking for
code, which are somewhat common. Selectors should be updated to cover
both code and erb:
selector: "code:contains('my_method'), erb:contains('my_method')"
|
@jhawthorn - thanks for tackling this, I'm not against this change I think it's something that's required to solve the issues you've outlined. I want to review this in detail, and get it merged. What do you think about dropping the erb- from the erb-loud and erb-silent. seems a little redundant to me? |
|
How does haml then get represented? |
|
@GeekOnCoffee haml gets converted to ERB before we remove the <% %> tags, so it's doesn't matter! |
|
I'm pro dropping the erb- from the erb erb-loud too. |
Might as well, since we are already breaking selectors. I can add that this evening. |
|
We should look at doing a new version of deface for this, as it's likely to break existing stores. |
|
oh sweet jesus YES! 💥 |
As we're breaking existing selectors anyways, we might as well remove the redundant erb-.
|
Merged, thanks for the contribution @jhawthorn @swrobel - any feedback on this with respect to jruby? @GeekOnCoffee - I agree, I've been holding back the 1.0.0 release until we got working jruby support so I think we could be ready for that now. I'm going to add some log output for people using the older style code erb-loud / erb-silent in their selectors to try and cut down the WTF per minute when people upgrade. |
|
I just pushed 1.0.0.rc4 |
|
Will test as soon as I have time and update |
|
All good here! 💥 |
|
@swrobel - happy days! |
|
@BDQ it seems like 1.0.0.rc4 is stable for me ... any 1.0.0 final plans? |
|
I added a separate issue, but I'll mention it here...can we get deface.heroku.com updated to offer the choice of 0.9 and 1.0 parsing? Thanks. |
This changes the ERB template parser to use
<erb>rather than<code>to designate code blocks in a template. This is a somewhat large change to functionality, but fixes several important issues which I can't imagine a better way to fix.For example:
Used to parse as:
But now parses as:
This fixes a number of problems that result from using
<code>:</code>tags in the original ERB, which would be turned into%>(Deface, Bootstrap and tag <code> #101)<code>tags in the<head>of a document would be moved to the body. Since<code>is a real HTML tag, and isn't allowed in the head, it will be moved into the body of the document. This happens in nokogiri under jRuby (Deface/JRuby closes head tag leaving head elements within the body #84) or libXML 2.9.0 (LibXML2 2.9.0+ problems #100).<erb>, being an invented tag, does not have this issue.Unfortunately, this breaks any existing deface selectors looking for code, which are somewhat common. To be backwards compatible, selectors could be updated to cover both code and erb:
UPDATE: Now uses
<erb loud>instead of<erb erb-loud>