791: "Publish Reply" notification banner appears on unmodified reviews following modified review
- Fixed
- Review Board
| timw.a******@gmai***** (Google Code) (Is this you? Claim this profile.) | |
 |
|
| Jan. 4, 2009 |
What's the URL of the page containing the problem? http://server/r/123/ What steps will reproduce the problem? 1. Find a review request with two or more reviews (at least one containing code comments, though that may not matter) 2. Write a reply to one of the review comments in one review, and save (but do not publish) it. Make sure this review is NOT the last review in the request. 3. Open the same review again (reload the URL, for example) What is the expected output? What do you see instead? Actual: After 2: the "Publish Reply" notification banner (green publish/discard box) appears above the replied-to review and also all reviews after it (but none before it). After 3: the same problem exists Expected: After 2: the "Publish Reply" notification banner (green publish/discard box) appears only above the replied-to review. After 3: same as after 2 What operating system are you using? What browser? Ubuntu 8.10's Firefox 3.0.5 on x86_64. Please provide any additional information below. This may be related to Issue 789 and Issue 790, if the reply box is strictly being shown by client-side javascript (in my opinion, the fact that both step 2 and 3 show the same behavior suggest to me that it's a client-side javascript issue).
Firebug = awesome... Cheat sheet: $.fn.commentSection = function(review_id, context_id, context_type) This gets called for each review box that is posted (seems to work fine, including receiving the correct review_id for each box). This function uses nested selectors to essentially do: find(div#review_id).find(div.banner).do_something Here's the catch: the page structure does not sync with the script's assumption. The structure actually looks like: review 1: div#content > div#reviewX > div.box-container... review 2: div#content > div#reviewX > div#reviewY > div.box-container... review 3: div#content > div#reviewX > div#reviewY > div#reviewZ > div.box-container... Essentially, this means find(div#reviewX).find(div.banner) yields all successive reviews' banners. I'll apologize now for taking my research this far and not posting a fix, but I haven't constructed one yet.
Since I'm the impatient type, I'm posting my findings in inline-patch-speak here
instead of as a review. Do with it what you will.
+++ ./templates/reviews/review_detail.html 2009-01-03 17:22:45.000000000 -0500
@@ -103,9 +103,10 @@
<pre class="body_bottom">{{entry.review.body_bottom|escape}}</pre>
{% reply_section entry.review "" "body_bottom" "rcbb" %}
{% endif %}
- </div>
-</div>
+ </div><!-- body -->
+</div><!-- main -->
{% endbox %}
+</div><!-- review -->
{% endif %}
{% if entry.changedesc %}
This fixes it for me locally, but I did not test beyond loading a single
reply-pending review.
Looks good. Thanks for doing the work on this. Fixed in r1667.
-
+ Fixed -
- Priority-Medium + Priority-Critical + Milestone-Release1.0 + Component-Reviews -
+ chipx86