1905: Extra whitespace is not highlighted

mikhail.********@gmai***** (Google Code) (Is this you? Claim this profile.)
Dec. 16, 2010
What version are you running?
ReviewBoard-1.5-py2.6 and up

What's the URL of the page containing the problem?
e.g. http://reviews.reviewboard.org/r/1914/diff/3/#index_header


What steps will reproduce the problem?
1. create review request, upload patch contained some extra whitespace
2. click "View diffs"

What is the expected output? What do you see instead?
ACT: only some of extra whitespaces are highlighted
whitespace in RBBugTracker/setup.py:15 is highlighted
whitespace in RBBugTracker/RBBugTracker/templates/googlecode/save_username.html:7 is not highlighted
EXP: all whitespaces are highlighted

What operating system are you using? What browser?
Chromium 6.0, Firefox 3.6.12, IE 8

Please provide any additional information below.

The bug occurs because toggleExtraWhitespace called in $(document).ready (diffviewer.js:1335)
toggleExtraWhitespace function toggles "ewhl" class on all whitespace elements. Since diffs content is loaded asynchronously, most of the whitespace elements are not loaded when document.ready occured.

In my install i fixed it following way:

diff --git a/reviewboard/htdocs/media/rb/js/diffviewer.js b/reviewboard/htdocs/media/rb/js/diffviewer.js
index d283d89..116b016 100644
--- a/reviewboard/htdocs/media/rb/js/diffviewer.js
+++ b/reviewboard/htdocs/media/rb/js/diffviewer.js
@@ -1325,7 +1325,7 @@ function toggleExtraWhitespace(init)
     }

     /* Toggle highlighting */
-    $("table.sidebyside .ew").toggleClass("ewhl");
+    $("#diffs").toggleClass("ewhl");

     /* Toggle the display of the button itself */
     $(".review-request ul.controls li.ew").toggle();
diff --git a/reviewboard/templates/diffviewer/view_diff.html b/reviewboard/templates/diffviewer/view_diff.html
index de1f8c2..62dafc8 100644
--- a/reviewboard/templates/diffviewer/view_diff.html
+++ b/reviewboard/templates/diffviewer/view_diff.html
@@ -23,7 +23,7 @@
 <link rel="stylesheet" type="text/css" href="{{MEDIA_URL}}rb/css/syntax.css?{{MEDIA_SERIAL}}" />
 {% if siteconfig.settings.diffviewer_show_trailing_whitespace|default_if_none:1 %}
 <style type="text/css">
-  .ewhl { background: #ee3434; }
+  #diffs.ewhl table.sidebyside .ew { background: #ee3434; }
 </style>
 {% endif %}
 {% endblock %}

Toggle "ewhl" class only on parent element.
ew elements are highlighted depending on this not their own class.
chipx86
#1 chipx86
Seems reasonable, from a cursory glance. Can you put this up on http://reviews.reviewboard.org/ ?
  • +PendingReview
  • +Component-DiffViewer
    +Milestone-Release1.5.x
#2 mikhail.********@gmai***** (Google Code) (Is this you? Claim this profile.)
Please see http://reviews.reviewboard.org/r/1960/
david
#3 david
Fixed in master as 60f93c0. Thanks!
  • -PendingReview
    +Fixed