Title: [136556] trunk/Websites/bugs.webkit.org
- Revision
- 136556
- Author
- o...@chromium.org
- Date
- 2012-12-04 13:11:01 -0800 (Tue, 04 Dec 2012)
Log Message
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:
Modified Paths
Diff
Modified: trunk/Websites/bugs.webkit.org/ChangeLog (136555 => 136556)
--- trunk/Websites/bugs.webkit.org/ChangeLog 2012-12-04 21:10:38 UTC (rev 136555)
+++ trunk/Websites/bugs.webkit.org/ChangeLog 2012-12-04 21:11:01 UTC (rev 136556)
@@ -1,3 +1,19 @@
+2012-12-04 Ojan Vafai <o...@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 <rn...@webkit.org>
committers-autocomplete.js works only with WebKit based browsers
Modified: trunk/Websites/bugs.webkit.org/code-review-test.html (136555 => 136556)
--- trunk/Websites/bugs.webkit.org/code-review-test.html 2012-12-04 21:10:38 UTC (rev 136555)
+++ trunk/Websites/bugs.webkit.org/code-review-test.html 2012-12-04 21:11:01 UTC (rev 136556)
@@ -228,7 +228,7 @@
crawlDiff();
appendToolbar();
- var line = document.getElementById('line0')
+ var line = document.getElementById('line0');
var author = "o...@chromium.org";
var comment_text = "This change sux.";
addPreviousComment(line, author, comment_text);
@@ -261,6 +261,49 @@
document.getElementById('diff-content').innerHTML = '';
}
+function testSideBySideDiffWithPreviousCommentsOnSharedLine() {
+ document.getElementById('diff-content').innerHTML =
+ '<div class="FileDiff">' +
+ '<h1><a href="" +
+ '<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: 'o...@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"));
Modified: trunk/Websites/bugs.webkit.org/code-review.js (136555 => 136556)
--- trunk/Websites/bugs.webkit.org/code-review.js 2012-12-04 21:10:38 UTC (rev 136555)
+++ trunk/Websites/bugs.webkit.org/code-review.js 2012-12-04 21:11:01 UTC (rev 136556)
@@ -66,7 +66,6 @@
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 = {};
@@ -89,20 +88,17 @@
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)));
- }
+ var i = 0;
+ var line;
+ do {
+ line = $('#' + idForLine(i));
+ if (line[0])
+ callback(line);
+ i++;
+ } while (line[0]);
}
- function idify() {
- this.id = nextLineID();
- }
-
function hoverify() {
$(this).hover(function() {
$(this).addClass('hot');
@@ -423,7 +419,7 @@
$(file).find(query).each(function() {
if ($(this).text() != line_number)
return;
- var line = $(this).parent();
+ var line = lineContainerFromDescendant($(this));
addPreviousComment(line, author, comment_text);
});
}
@@ -601,6 +597,11 @@
}
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');
@@ -1928,6 +1929,10 @@
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'))
@@ -2037,6 +2042,8 @@
window.addPreviousComment = addPreviousComment;
window.tracLinks = tracLinks;
window.crawlDiff = crawlDiff;
+ window.convertAllFileDiffs = convertAllFileDiffs;
+ window.displayPreviousComments = displayPreviousComments;
window.discardComment = discardComment;
window.addCommentField = addCommentField;
window.acceptComment = acceptComment;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-changes
