https://bugs.webkit.org/show_bug.cgi?id=104025
Reviewed by Adam Barth.
If we side-by-sidify a shared diff line, and then apply
a previous comment, we would incorrectly put the comment
on the Line instead of the LineContainer.
Also, get rid of global next_line_id to simplify testing.
* code-review-test.html:
* code-review.js:
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@136556
268f45cc-cd09-0410-ab3c-
d52691b4dbfc
+2012-12-04 Ojan Vafai <ojan@chromium.org>
+
+ Can't add followup comment to a previous comment
+ https://bugs.webkit.org/show_bug.cgi?id=104025
+
+ Reviewed by Adam Barth.
+
+ If we side-by-sidify a shared diff line, and then apply
+ a previous comment, we would incorrectly put the comment
+ on the Line instead of the LineContainer.
+
+ Also, get rid of global next_line_id to simplify testing.
+
+ * code-review-test.html:
+ * code-review.js:
+
2012-11-06 Ryosuke Niwa <rniwa@webkit.org>
committers-autocomplete.js works only with WebKit based browsers
crawlDiff();
appendToolbar();
- var line = document.getElementById('line0')
+ var line = document.getElementById('line0');
var author = "ojan@chromium.org";
var comment_text = "This change sux.";
addPreviousComment(line, author, comment_text);
document.getElementById('diff-content').innerHTML = '';
}
+function testSideBySideDiffWithPreviousCommentsOnSharedLine() {
+ document.getElementById('diff-content').innerHTML =
+ '<div class="FileDiff">' +
+ '<h1><a href="http://trac.webkit.org/browser/trunk/Source/WebCore/ChangeLog">Source/WebCore/ChangeLog</a></h1>' +
+ '<div class="DiffSection">' +
+ '<div class="DiffBlock">' +
+ '<div class="DiffBlockPart shared">' +
+ '<div class="Line LineContainer">' +
+ '<span class="from lineNumber">336</span><span class="to lineNumber">338</span><span class="text"> layoutFlexItems(*m_orderIterator, lineContexts);</span>' +
+ '</div>' +
+ '<div class="Line LineContainer">' +
+ '<span class="from lineNumber">337</span><span class="to lineNumber">339</span><span class="text"></span>' +
+ '</div>' +
+ '<div class="Line LineContainer">' +
+ '<span class="from lineNumber">338</span><span class="to lineNumber">340</span><span class="text"> LayoutUnit oldClientAfterEdge = clientLogicalBottom();</span>' +
+ '</div>' +
+ '</div><div class="clear_float">' +
+ '</div>' +
+ '</div>' +
+ '</div>';
+
+ next_line_id = 0;
+ eraseDraftComments();
+ crawlDiff();
+
+ convertAllFileDiffs('sidebyside', $('.FileDiff'));
+
+ displayPreviousComments([{
+ author: 'ojan@chromium.org',
+ file_name: 'Source/WebCore/ChangeLog',
+ line_number: 338,
+ comment_text: 'This change sux.'
+ }]);
+
+ var previous_comment = document.querySelector('.previousComment');
+ ASSERT_EQUAL(previous_comment.getAttribute('data-comment-for'), 'line0');
+
+ var new_comment = addCommentField(previous_comment);
+ ASSERT("New comment should exist and contain a textarea.", new_comment.find('textarea'));
+
+ document.getElementById('diff-content').innerHTML = '';
+}
+
function testIsChangeLog() {
ASSERT("Top-level ChangeLog file is a ChangeLog file", isChangeLog("ChangeLog"));
ASSERT("Second-level ChangeLog file is a ChangeLog file", isChangeLog("Tools/ChangeLog"));
var minLeftSideRatio = 10;
var maxLeftSideRatio = 90;
var file_diff_being_resized = null;
- var next_line_id = 0;
var files = {};
var original_file_contents = {};
var patched_file_contents = {};
return 'line' + number;
}
- function nextLineID() {
- return idForLine(next_line_id++);
- }
-
function forEachLine(callback) {
- for (var i = 0; i < next_line_id; ++i) {
- callback($('#' + idForLine(i)));
- }
- }
-
- function idify() {
- this.id = nextLineID();
+ var i = 0;
+ var line;
+ do {
+ line = $('#' + idForLine(i));
+ if (line[0])
+ callback(line);
+ i++;
+ } while (line[0]);
}
function hoverify() {
$(file).find(query).each(function() {
if ($(this).text() != line_number)
return;
- var line = $(this).parent();
+ var line = lineContainerFromDescendant($(this));
addPreviousComment(line, author, comment_text);
});
}
}
function crawlDiff() {
+ var next_line_id = 0;
+ var idify = function() {
+ this.id = idForLine(next_line_id++);
+ }
+
$('.Line').each(idify).each(hoverify);
$('.FileDiff').each(function() {
var header = $(this).children('h1');
return descendant.hasClass('Line') ? descendant : descendant.parents('.Line');
}
+ function lineContainerFromDescendant(descendant) {
+ return descendant.hasClass('LineContainer') ? descendant : descendant.parents('.LineContainer');
+ }
+
function lineFromLineContainer(lineContainer) {
var line = $(lineContainer);
if (!line.hasClass('Line'))
window.addPreviousComment = addPreviousComment;
window.tracLinks = tracLinks;
window.crawlDiff = crawlDiff;
+ window.convertAllFileDiffs = convertAllFileDiffs;
+ window.displayPreviousComments = displayPreviousComments;
window.discardComment = discardComment;
window.addCommentField = addCommentField;
window.acceptComment = acceptComment;