Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

docs(errorDisplay): encode < and > in error messages #14033

Conversation

gkalpak
Copy link
Member

@gkalpak gkalpak commented Feb 13, 2016

When an error message contains an HTML string (e.g. $location:nobase containing <base>), it was interpreted as a literal HTML element, instead of text. Error messages are not expected to render as HTML, but we still need to use .html() in errorDisplay, so that the links created by errorLinkFilter are properly displayed.
This commit solves this issue by replacing </> with &lt;/&gt;.

Related to #14016.

There is also another commit that adds some tests for the errors module of the docs app.

When an error message contains an HTML string (e.g. `$location:nobase` containing `<base>`), it was
interpreted as a literal HTML element, instead of text. Error messages are not expected to render
as HTML, but we still need to use `.html()` in `errorDisplay`, so that the links created by
`errorLinkFilter` are properly displayed.
This commit solves this issue by replacing `<`/`>` with `&lt;`/`&gt;`.

Related to angular#14016.
@gkalpak
Copy link
Member Author

gkalpak commented Feb 13, 2016

Before:
location nobase before

After:
location nobase after

@@ -34,6 +34,10 @@ angular.module('errors', ['ngSanitize'])


.directive('errorDisplay', ['$location', 'errorLinkFilter', function ($location, errorLinkFilter) {
var encodeAngularBrackets = function (text) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

encodeAngularBrackets -> encodeHtmlBrackets ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By Angular, @gkalpak means < and > right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think "angle brackets" might be more "correct" (and certainly less confusing).

@petebacondarwin
Copy link
Contributor

This looks good but I wonder if we could save some complexity by simply using a binding (which should do the escaping for us) rather than writing to element.html?

@gkalpak
Copy link
Member Author

gkalpak commented Feb 16, 2016

@petebacondarwin, we need the HTML (because we want to render <a> elements), so not sure what you have in mind.

TBH, a much simpler workaround would be to change the error message to not include <base> (but just base for example), but who likes workarounds :)

@gkalpak
Copy link
Member Author

gkalpak commented Feb 29, 2016

Merged as 321180a and 9c7c494.

@gkalpak gkalpak closed this Feb 29, 2016
@petebacondarwin
Copy link
Contributor

Nice thanks

@gkalpak gkalpak deleted the docs-guide-error-fix-display-of-angular-brackets branch March 1, 2016 12:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants