- Bugs:
-
- Groups:
- Description:
-
~ fixed in review.js:removeCommentFormIfEmpty: changed to 'if (value.strip() != '')',
~ fixed in review.js:removeCommentFormIfEmpty: stripTags() is no longer used in version 2.0, stripTags() wipes out the comment if the comment text only contains XML or html
- Testing Done:
-
I manually tried to recreate the bug with different ways and passed
+ + when inserting <sim realtime="true> or aaa <sim realtime="true>
+ + the comment get added and the bug doesnt occure
- Groups:
- Description:
-
~ fixed in review.js:removeCommentFormIfEmpty: stripTags() is no longer used in version 2.0, stripTags() wipes out the comment if the comment text only contains XML or html
~ fixed in review.js:removeCommentFormIfEmpty: stripTags() is no longer used in version 2.0,
+ stripTags() wipes out the xml tags from the comment; it removes everything between '<' and '>' - Testing Done:
-
~ I manually tried to recreate the bug with different ways and passed
~ ~ I manually tried to recreate the bug with different ways and passed
~ by submitting '<sim realtime="true>' or 'something <sim realtime="true>' in the comments - when inserting <sim realtime="true> or aaa <sim realtime="true>
- - the comment get added and the bug doesnt occure
-
I'd like to make some suggestions about the review request text. We use this text as the commit message when pushing changes, so it's important that it be well-formed and easy to read. Getting in this practice will be good for the future, too :)
The summary should be less about the implementation and more about the higher-level functionality. Something like "Fix stripping of XML-like syntax in comments"
The description is currently OK at explaining what the bug was (on a technical level) and why your change fixes it, but it's not very easy to understand. I would do something more like this:
"If a comment consisted of only XML-like syntax (such as "<test>"), it would be treated as if the comment was completely empty and the comment would be canceled. The issue here is that reviewCommentFormIsEmpty would strip out tags before testing whether the comment was empty or not. Removing the call to stripTags() fixes the bug."
Please also be careful--your linked bug number is incorrect.
- Summary:
-
fixed bug 3291 .. review.js:removeCommentFormIfEmpty: changed to 'if (value.strip() != '')'Fix stripping of XML-like syntax in comments
- Description:
-
~ fixed in review.js:removeCommentFormIfEmpty: stripTags() is no longer used in version 2.0,
~ If a comment consisted of only XML-like syntax (such as "<test>"), it would be treated as if the comment was completely empty and the comment would be canceled. The issue here is that reviewCommentFormIsEmpty would strip out tags before testing whether the comment was empty or not. Removing the call to stripTags() fixes the bug.
- stripTags() wipes out the xml tags from the comment; it removes everything between '<' and '>' - Testing Done:
-
~ I manually tried to recreate the bug with different ways and passed
~ by submitting '<sim realtime="true>' or 'something <sim realtime="true>' in the comments ~ I manually tried to recreate the bug with different ways.
~ By submitting '<sim realtime="true>' or 'something <sim realtime="true>' in the comments you can verify that the bug is no longer exists.