ErbRenderer should always return an HTML-safe String #2098

floehopper · 2015-11-16T17:41:53Z

Content that has already been rendered through ErbRenderer doesn't need to be HTML-escaped, because ActionView::Base will have already taken care of that. So we can mark content returned from ErbRenderer as being html_safe.

chrisroos · 2015-11-17T10:51:15Z

Looks good to me.

The fact that all content has been rendered by an instance of `ActionView::Base` means that it's safe to assume it's HTML-safe. `GovspeakPresenter#html` was already being marking its output as HTML-safe, but non-Govspeak content was not being marked as such. It would probably make sense to move the call to `String#html_safe` in `GovspeakPresenter#html` into `ErbRenderer#content_for`, but it's used in quite a few places and I want to reduce the scope of this change, so this is a good first step.

The main purpose of this is so that we can move the call to `String#chomp` into the `ErbRenderer` in order to mark the result as HTML-safe. In the long run, we could probably do without the call to `String#chomp` altogether, but the trouble is that it would cause changes to most/all of the regression test artefacts.

@chrisroos

The page `title` and `h1` elements on all question pages is obtained from the start node title. This is now *correctly* being marked as HTML-safe when it comes back from the `ErbRenderer` and so the apostrophes are no longer being HTML-escaped. Note that @chrisroos & I were confused for a while because the old HTML-escaped behaviour was not visible in development or production. We're pretty sure this is because Slimmer uses Nokogiri to extract a fragment of HTML from the page and then re-inserts it in a new "layout". We suspect this process does some kind of normalisation which means the escaped apostrophes are unescaped.

floehopper · 2015-11-17T12:14:56Z

I've rebased this against master and force-pushed in preparation for merging.

…urn-html-safe-string ErbRenderer should always return an HTML-safe String

Since #2098 & #2112, apostrophes rendered via `SmartAnswer::ErbRenderer` have not been HTML-escaped. Now that we are rendering question content via this renderer, we need to update the regression test artefacts for question pages accordingly. I ran the following commands to do this: $ find test/artefacts/benefit-cap-calculator -name '*.html' | xargs sed -i .bak "s/'/'/g" $ git clean -fd test/artefacts/benefit-cap-calculator/**/*.bak

Since #2098 & #2112, apostrophes rendered via `SmartAnswer::ErbRenderer` have not been HTML-escaped. Now that we are rendering question content via this renderer, we need to update the regression test artefacts for question pages accordingly. I ran the following commands to do this: $ find test/artefacts/am-i-getting-minimum-wage -name '*.html' | xargs sed -i .bak "s/'/'/g" $ git clean -fd test/artefacts/am-i-getting-minimum-wage/**/*.bak

Since #2098 & #2112, apostrophes rendered via `SmartAnswer::ErbRenderer` have not been HTML-escaped. Now that we are rendering question content via this renderer, we need to update the regression test artefacts for question pages accordingly. I ran the following commands to do this: $ find test/artefacts/benefit-cap-calculator -name '*.html' | xargs sed -i .bak "s/'/'/g" $ git clean -fd test/artefacts/benefit-cap-calculator/**/*.bak

Since #2098 & #2112, apostrophes rendered via `SmartAnswer::ErbRenderer` have not been HTML-escaped. Now that we are rendering question content via this renderer, we need to update the regression test artefacts for question pages accordingly. I ran the following commands to do this: $ find test/artefacts/calculate-married-couples-allowance -name '*.html' | xargs sed -i .bak "s/'/'/g" $ git clean -fd test/artefacts/calculate-married-couples-allowance/**/*.bak

Since #2098 & #2112, apostrophes rendered via `SmartAnswer::ErbRenderer` have not been HTML-escaped. Now that we are rendering question content via this renderer, we need to update the regression test artefacts for question pages accordingly. I ran the following commands to do this: $ find test/artefacts/calculate-your-child-maintenance -name '*.html' | xargs sed -i .bak "s/'/'/g" $ git clean -fd test/artefacts/calculate-your-child-maintenance/**/*.bak

Since #2098 & #2112, apostrophes rendered via `SmartAnswer::ErbRenderer` have not been HTML-escaped. Now that we are rendering question content via this renderer, we need to update the regression test artefacts for question pages accordingly. I ran the following commands to do this: $ find test/artefacts/benefit-cap-calculator -name '*.html' | xargs sed -i .bak "s/'/'/g" $ git clean -fd test/artefacts/benefit-cap-calculator/**/*.bak

Since #2098 & #2112, apostrophes rendered via `SmartAnswer::ErbRenderer` have not been HTML-escaped. Now that we are rendering question content via this renderer, we need to update the regression test artefacts for question pages accordingly. I ran the following commands to do this: $ find test/artefacts/am-i-getting-minimum-wage -name '*.html' | xargs sed -i .bak "s/'/'/g" $ git clean -fd test/artefacts/am-i-getting-minimum-wage/**/*.bak

Since #2098 & #2112, apostrophes rendered via `SmartAnswer::ErbRenderer` have not been HTML-escaped. Now that we are rendering question content via this renderer, we need to update the regression test artefacts for question pages accordingly. I ran the following commands to do this: $ find test/artefacts/calculate-married-couples-allowance -name '*.html' | xargs sed -i .bak "s/'/'/g" $ git clean -fd test/artefacts/calculate-married-couples-allowance/**/*.bak

Since #2098 & #2112, apostrophes rendered via `SmartAnswer::ErbRenderer` have not been HTML-escaped. Now that we are rendering question content via this renderer, we need to update the regression test artefacts for question pages accordingly. I ran the following commands to do this: $ find test/artefacts/calculate-your-holiday-entitlement -name '*.html' | xargs sed -i .bak "s/'/'/g" $ git clean -fd test/artefacts/calculate-your-holiday-entitlement/**/*.bak

Since #2098 & #2112, apostrophes rendered via `SmartAnswer::ErbRenderer` have not been HTML-escaped. Now that we are rendering question content via this renderer, we need to update the regression test artefacts for question pages accordingly. I ran the following commands to do this: $ find test/artefacts/check-uk-visa -name '*.html' | xargs sed -i .bak "s/'/'/g" $ git clean -fd test/artefacts/check-uk-visa/**/*.bak

Since #2098 & #2112, apostrophes rendered via `SmartAnswer::ErbRenderer` have not been HTML-escaped. Now that we are rendering question content via this renderer, we need to update the regression test artefacts for question pages accordingly. I ran the following commands to do this: $ find test/artefacts/childcare-costs-for-tax-credits -name '*.html' | xargs sed -i .bak "s/'/'/g" $ git clean -fd test/artefacts/childcare-costs-for-tax-credits/**/*.bak

Since #2098 & #2112, apostrophes rendered via `SmartAnswer::ErbRenderer` have not been HTML-escaped. Now that we are rendering question content via this renderer, we need to update the regression test artefacts for question pages accordingly. I ran the following commands to do this: $ find test/artefacts/estimate-self-assessment-penalties -name '*.html' | xargs sed -i .bak "s/&/\&/g" $ git clean -fd test/artefacts/**/*.bak

Since #2098 & #2112, apostrophes rendered via `SmartAnswer::ErbRenderer` have not been HTML-escaped. Now that we are rendering question content via this renderer, we need to update the regression test artefacts for question pages accordingly. I ran the following commands to do this: $ find test/artefacts/inherits-someone-dies-without-will -name '*.html' | xargs sed -i .bak "s/'/'/g" $ git clean -fd test/artefacts/inherits-someone-dies-without-will/**/*.bak

Since #2098 & #2112, ampersands rendered via `SmartAnswer::ErbRenderer` have not been HTML-escaped. Now that we are rendering question content via this renderer, we need to update the content in the ERB template to use the HTML entity for ampersand.

Since #2098 & #2112, apostrophes rendered via `SmartAnswer::ErbRenderer` have not been HTML-escaped. Now that we are rendering question content via this renderer, we need to update the regression test artefacts for question pages accordingly. I ran the following commands to do this: $ find test/artefacts/pip-checker -name '*.html' -print0 | xargs -0 sed -i .bak "s/'/'/g" $ git clean -fd test/artefacts/pip-checker/**/*.bak

Since #2098 & #2112, apostrophes rendered via `SmartAnswer::ErbRenderer` have not been HTML-escaped. Now that we are rendering question content via this renderer, we need to update the regression test artefacts for question pages accordingly. I ran the following commands to do this: $ find test/artefacts/plan-adoption-leave -name '*.html' -print0 | xargs -0 sed -i .bak "s/'/'/g" $ git clean -fd test/artefacts/plan-adoption-leave/**/*.bak

Since #2098 & #2112, apostrophes rendered via `SmartAnswer::ErbRenderer` have not been HTML-escaped. Now that we are rendering question content via this renderer, we need to update the regression test artefacts for question pages accordingly. I ran the following commands to do this: $ find test/artefacts/simplified-expenses-checker -name '*.html' -print0 | xargs -0 sed -i .bak "s/'/'/g" $ git clean -fd test/artefacts/simplified-expenses-checker/**/*.bak

Since #2098 & #2112, ampersands rendered via `SmartAnswer::ErbRenderer` have not been HTML-escaped. Now that we are rendering question content via this renderer, we need to update the content in the ERB template to use the HTML entity for ampersand.

Since #2098 & #2112, apostrophes rendered via `SmartAnswer::ErbRenderer` have not been HTML-escaped. Now that we are rendering question content via this renderer, we need to update the regression test artefacts for question pages accordingly. I ran the following commands to do this: $ find test/artefacts/student-finance-calculator -name '*.html' -print0 | xargs -0 sed -i .bak "s/'/'/g" $ git clean -fd test/artefacts/student-finance-calculator/**/*.bak

Since #2098 & #2112, apostrophes rendered via `SmartAnswer::ErbRenderer` have not been HTML-escaped. Now that we are rendering question content via this renderer, we need to update the regression test artefacts for question pages accordingly. I ran the following commands to do this: $ find test/artefacts/calculate-your-child-maintenance -name '*.html' | xargs sed -i .bak "s/'/'/g" $ git clean -fd test/artefacts/calculate-your-child-maintenance/**/*.bak

Since #2098 & #2112, apostrophes rendered via `SmartAnswer::ErbRenderer` have not been HTML-escaped. Now that we are rendering question content via this renderer, we need to update the regression test artefacts for question pages accordingly. I ran the following commands to do this: $ find test/artefacts/calculate-your-holiday-entitlement -name '*.html' | xargs sed -i .bak "s/'/'/g" $ git clean -fd test/artefacts/calculate-your-holiday-entitlement/**/*.bak

Since #2098 & #2112, apostrophes rendered via `SmartAnswer::ErbRenderer` have not been HTML-escaped. Now that we are rendering question content via this renderer, we need to update the regression test artefacts for question pages accordingly. I ran the following commands to do this: $ find test/artefacts/check-uk-visa -name '*.html' | xargs sed -i .bak "s/'/'/g" $ git clean -fd test/artefacts/check-uk-visa/**/*.bak

Since #2098 & #2112, apostrophes rendered via `SmartAnswer::ErbRenderer` have not been HTML-escaped. Now that we are rendering question content via this renderer, we need to update the regression test artefacts for question pages accordingly. I ran the following commands to do this: $ find test/artefacts/childcare-costs-for-tax-credits -name '*.html' | xargs sed -i .bak "s/'/'/g" $ git clean -fd test/artefacts/childcare-costs-for-tax-credits/**/*.bak

Since #2098 & #2112, ampersands rendered via `SmartAnswer::ErbRenderer` have not been HTML-escaped. Now that we are rendering question content via this renderer, we need to update the content in the ERB template to use the HTML entity for ampersand.

Since #2098 & #2112, apostrophes rendered via `SmartAnswer::ErbRenderer` have not been HTML-escaped. Now that we are rendering question content via this renderer, we need to update the regression test artefacts for question pages accordingly. I ran the following commands to do this: $ find test/artefacts/uk-benefits-abroad -name '*.html' -print0 | xargs -0 sed -i .bak "s/'/'/g" $ git clean -fd test/artefacts/uk-benefits-abroad/**/*.bak

Since #2098 & #2112, apostrophes rendered via `SmartAnswer::ErbRenderer` have not been HTML-escaped. Now that we are rendering question content via this renderer, we need to update the regression test artefacts for question pages accordingly. I ran the following commands to do this: $ find test/artefacts/inherits-someone-dies-without-will -name '*.html' | xargs sed -i .bak "s/'/'/g" $ git clean -fd test/artefacts/inherits-someone-dies-without-will/**/*.bak

Since #2098 & #2112, apostrophes rendered via `SmartAnswer::ErbRenderer` have not been HTML-escaped. Now that we are rendering question content via this renderer, we need to update the regression test artefacts for question pages accordingly. I ran the following commands to do this: $ find test/artefacts/uk-benefits-abroad -name '*.html' -print0 | xargs -0 sed -i .bak "s/'/'/g" $ git clean -fd test/artefacts/uk-benefits-abroad/**/*.bak

Since #2098 & #2112, apostrophes rendered via `SmartAnswer::ErbRenderer` have not been HTML-escaped. Now that we are rendering question content via this renderer, we need to update the regression test artefacts for question pages accordingly. I ran the following commands to do this: $ find test/artefacts/pip-checker -name '*.html' -print0 | xargs -0 sed -i .bak "s/'/'/g" $ git clean -fd test/artefacts/pip-checker/**/*.bak

Since #2098 & #2112, apostrophes rendered via `SmartAnswer::ErbRenderer` have not been HTML-escaped. Now that we are rendering question content via this renderer, we need to update the regression test artefacts for question pages accordingly. I ran the following commands to do this: $ find test/artefacts/plan-adoption-leave -name '*.html' -print0 | xargs -0 sed -i .bak "s/'/'/g" $ git clean -fd test/artefacts/plan-adoption-leave/**/*.bak

Since #2098 & #2112, apostrophes rendered via `SmartAnswer::ErbRenderer` have not been HTML-escaped. Now that we are rendering question content via this renderer, we need to update the regression test artefacts for question pages accordingly. I ran the following commands to do this: $ find test/artefacts/simplified-expenses-checker -name '*.html' -print0 | xargs -0 sed -i .bak "s/'/'/g" $ git clean -fd test/artefacts/simplified-expenses-checker/**/*.bak

Since #2098 & #2112, ampersands rendered via `SmartAnswer::ErbRenderer` have not been HTML-escaped. Now that we are rendering question content via this renderer, we need to update the content in the ERB template to use the HTML entity for ampersand.

Since #2098 & #2112, apostrophes rendered via `SmartAnswer::ErbRenderer` have not been HTML-escaped. Now that we are rendering question content via this renderer, we need to update the regression test artefacts for question pages accordingly. I ran the following commands to do this: $ find test/artefacts/maternity-paternity-calculator -name '*.html' -print0 | xargs -0 sed -i .bak "s/'/'/g" $ git clean -fd test/artefacts/maternity-paternity-calculator/**/*.bak

Since #2098 & #2112, double-quotes rendered via `SmartAnswer::ErbRenderer` have not been HTML-escaped. Now that we are rendering question content via this renderer, we need to update the regression test artefacts for question pages accordingly. I ran the following commands to do this: $ find test/artefacts/maternity-paternity-calculator -name '*.html' -print0 | xargs -0 sed -i .bak "s/"/\"/g" $ git clean -fd test/artefacts/maternity-paternity-calculator/**/*.bak

Since #2098 & #2112, apostrophes rendered via `SmartAnswer::ErbRenderer` have not been HTML-escaped. Now that we are rendering question content via this renderer, we need to update the regression test artefacts for question pages accordingly. I ran the following commands to do this: $ find test/artefacts/student-finance-calculator -name '*.html' -print0 | xargs -0 sed -i .bak "s/'/'/g" $ git clean -fd test/artefacts/student-finance-calculator/**/*.bak

Since #2098 & #2112, apostrophes rendered via `SmartAnswer::ErbRenderer` have not been HTML-escaped. Now that we are rendering question content via this renderer, we need to update the regression test artefacts for question pages accordingly. I ran the following commands to do this: $ find test/artefacts/uk-benefits-abroad -name '*.html' -print0 | xargs -0 sed -i .bak "s/'/'/g" $ git clean -fd test/artefacts/uk-benefits-abroad/**/*.bak

Since #2098 & #2112, apostrophes rendered via `SmartAnswer::ErbRenderer` have not been HTML-escaped. Now that we are rendering question content via this renderer, we need to update the regression test artefacts for question pages accordingly. I ran the following commands to do this: $ find test/artefacts/maternity-paternity-calculator -name '*.html' -print0 | xargs -0 sed -i .bak "s/'/'/g" $ git clean -fd test/artefacts/maternity-paternity-calculator/**/*.bak

Since #2098 & #2112, double-quotes rendered via `SmartAnswer::ErbRenderer` have not been HTML-escaped. Now that we are rendering question content via this renderer, we need to update the regression test artefacts for question pages accordingly. I ran the following commands to do this: $ find test/artefacts/maternity-paternity-calculator -name '*.html' -print0 | xargs -0 sed -i .bak "s/"/\"/g" $ git clean -fd test/artefacts/maternity-paternity-calculator/**/*.bak

chrisroos self-assigned this Nov 17, 2015

chrisroos added the LGTM label Nov 17, 2015

floehopper added 3 commits November 17, 2015 12:14

floehopper force-pushed the erb-renderer-should-always-return-html-safe-string branch from 8831576 to 8307d7f Compare November 17, 2015 12:14

floehopper added a commit that referenced this pull request Nov 17, 2015

Merge pull request #2098 from alphagov/erb-renderer-should-always-ret…

89001e4

…urn-html-safe-string ErbRenderer should always return an HTML-safe String

floehopper merged commit 89001e4 into master Nov 17, 2015

floehopper deleted the erb-renderer-should-always-return-html-safe-string branch November 17, 2015 12:18

floehopper mentioned this pull request Nov 18, 2015

ErbRenderer should always return HTML-safe string for option labels #2112

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ErbRenderer should always return an HTML-safe String #2098

ErbRenderer should always return an HTML-safe String #2098

floehopper commented Nov 16, 2015

chrisroos commented Nov 17, 2015

floehopper commented Nov 17, 2015

ErbRenderer should always return an HTML-safe String #2098

ErbRenderer should always return an HTML-safe String #2098

Conversation

floehopper commented Nov 16, 2015

chrisroos commented Nov 17, 2015

floehopper commented Nov 17, 2015