Add input escaping to the bugs section of review

Review Request #5774 — Created May 5, 2014 and submitted

Information

Review Board

Reviewers

If you create a new review and type a string that should be escaped into the bugs secion (e.g. <A>), it disappears. Can be fixed by using view.formatText that escapes input.
Note that even though the text that you entered isn't displayed, if you refresh the review, it's there, and is displayed, as the template has "|safe" in it. The problem is only visible after immediate value update.

There are a couple of other fields that might have the same issue, but none of them (depends on, target groups) are likely to have input that should be escaped in them.

Before:
1) Created a new review.
2) Click edit on bug description.
3) Enter "<A>" into the field and press enter.
4) Text disappears.

After:
1) Created a new review.
2) Click edit on bug description.
3) Enter "<A>" into the field and press enter.
4) Text is there, just as expected.

Also tested behaviour with multiple inputs like "<123> <456>".

Description From Last Updated

I think this doesn't do the right thing. urlizeList will add in <a> tags to make links to the given …

daviddavid

This will potentially run the linkify methods, which doesn't seem appropriate here. I think probably we can just change this …

daviddavid
david
  1. 
      
  2. I think this doesn't do the right thing. urlizeList will add in <a> tags to make links to the given items, but then formatText will escape the whole thing.

    Probably a better solution is to leave the outer stuff as-is, but have the inner method return _.escape(bugTrackerURL.replace('%s', item)).

    Can you create a repository with an associated bug tracker and test if linked bugs work appropriately?

    1. This didn't seem to work, but since the problem is with the text between the <a> and </a> (the link itself is correct), I just passed escape method to the urlizeList function that applies it to the text between <a> and </a>.

      I tested it with reviewboards bugtracker, seems to work: numbers are still displayed and have correct links, and things like "<A>" are displayed correctly and have "correct" links (though they are meaningless).

  3. This will potentially run the linkify methods, which doesn't seem appropriate here. I think probably we can just change this to use $el.text(...);

  4. 
      
VO
david
  1. Ship It!

  2. 
      
VO
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-2.0.x (accf5f8)
Loading...