Attachment #243435 for bug #133939

View Details Raw Unified Return to Bug 133939

| Differences between



2014-12-16  Grzegorz Czajkowski  <g.czajkowski@gmail.com>

        shouldBecomeEqual() behaves as shouldBe() if the testing expression returns the expected value
        https://bugs.webkit.org/show_bug.cgi?id=133939

        Reviewed by NOBODY (OOPS!).

        Since programmatically selection change triggers spellchecker asynchronously
        call updateEditorUINowIfScheduled() in Internals::hasGrammarMarker() to apply
        pending document markers.
        Internals::hasSpellingMarker() already does it.

        No new tests, covered by editing/spelling/grammar-edit-word.html

        * testing/Internals.cpp:
        (WebCore::Internals::hasGrammarMarker):
        Call updateEditorUINowIfScheduled() to update document markers.

2014-12-17  Zan Dobersek  <zdobersek@igalia.com>

        [TexMap] Sprinkle range-based for-loops where still possible

Lines 1563-1568 bool Internals::hasGrammarMarker(int from, int length, ExceptionCode&) a/Source/WebCore/testing/Internals.cpp_sec1
- a/Source/WebCore/testing/Internals.cpp +2 lines
1563	if (!document \|\| !document->frame())	1563	if (!document \|\| !document->frame())
1564	return 0;	1564	return 0;
1565		1565
		1566	updateEditorUINowIfScheduled();
		1567
1566	return document->frame()->editor().selectionStartHasMarkerFor(DocumentMarker::Grammar, from, length);	1568	return document->frame()->editor().selectionStartHasMarkerFor(DocumentMarker::Grammar, from, length);
1567	}	1569	}
1568		1570



2014-12-15  Grzegorz Czajkowski  <g.czajkowski@samsung.com>

        shouldBecomeEqual() behaves as shouldBe() if the testing expression returns the expected value
        https://bugs.webkit.org/show_bug.cgi?id=133939

        Reviewed by NOBODY (OOPS!).

        First call of eval(expression) inside shouldBecomeEqual was always
        made synchronously. If the testing expression returns the same value
        as expected one then shouldBecomeEqual() will immediately report PASS,
        for example,

        shouldBecomeEqual(internals.hasSpellingMarker("wellcome"), false);

        in consequence, assuming asynchronous path of spellchecking, spelling markers
        may appear after a while.

        The bug was caused by checking a condition at the beginning of shouldBecomeEqual(),
        before calling a timer. As a result, queued asynchronous events doesn't effect
        this checking.

        * editing/spelling/editing-multiple-words-with-markers.html:
        Since this tests if spelling markers do not appear after a selection change
        after r164401 we need to call updateEditorUINowIfScheduled(). Without it editor
        would report old position of markers. Additionally, add timeout to make things
        testable in the browser.
        Without proposed change in shouldBecomeEqual, this bug was not revealed.

        * editing/spelling/script-tests/spelling-backspace-between-lines.js:
        Replace magic number 100 with newly defined value.
        * editing/spelling/spellcheck-attribute.html: Ditto.
        * editing/spelling/spelling-backspace-between-lines.html: Ditto.

        * editing/spelling/grammar-edit-word.html: Ditto.
        The reason why updateEditorUINowIfScheduled() is not mandatory here is that
        the test uses editing commands which already call updateEditorUINowIfScheduled().
        However, timeout needs to be added as while grammar checking
        Editor::respondToChangedSelection() is invoked in some circumstances
        and causes asynchronous spellchecker updates.
        This is probably a new bug that was not revealed yet.

        * editing/spelling/resources/util.js: Ditto.
        Introduce the timeout value for pending selection updates that are
        required by spellchecker.

        * resources/js-test-pre.js:
        (shouldBecomeEqual):
        (shouldBecomeDifferent):
        Always check a condition on timer.

2014-12-16  Shivakumar JM  <shiva.jm@samsung.com>

        text node should not be created, On setting document.title to the empty string.

Lines 1-5 a/LayoutTests/editing/spelling/editing-multiple-words-with-markers.html_sec1
- a/LayoutTests/editing/spelling/editing-multiple-words-with-markers.html -1 / +4 lines
1	<html>	1	<html>
2	<head>	2	<head>
		3	<script src="resources/util.js"></script>
3	<script src="../editing.js"></script>	4	<script src="../editing.js"></script>
4	<title>Editing multiple words with markers test</title>	5	<title>Editing multiple words with markers test</title>
5	</head>	6	</head>
Lines 67-73 function checkSpellingMarkerAfterDeleteingSelection(selectionRange, spellingMark a/LayoutTests/editing/spelling/editing-multiple-words-with-markers.html_sec2
67	resetText();	68	resetText();
68	if (selectionRange) {	69	if (selectionRange) {
69	document.getElementById('testElement').setSelectionRange(selectionRange[0], selectionRange[1]);	70	document.getElementById('testElement').setSelectionRange(selectionRange[0], selectionRange[1]);
70	execDeleteCommand();	71	if (window.internals)
		72	internals.updateEditorUINowIfScheduled();
		73	setTimeout(execDeleteCommand, timeoutToApplyPendingProgramaticSelectionChanges);
71	}	74	}
72		75
73	spellingPositions = spellingMarkerPositions;	76	spellingPositions = spellingMarkerPositions;



</head> 
<body>
<script src="../../resources/js-test-pre.js"></script>
<script src="resources/util.js"></script>
<div id="src" contenteditable="true" spellcheck="true"></div><br/>
<script language="javascript">
description('Test if WebKit removes grammar markers when we edit a grammatically-incorrect word. '

    internals.settings.setAsynchronousSpellCheckingEnabled(true);
}

function editSentenceAsyncAndCheckMarker() {
    if (target.innerText != "You ha") {
    debug('Delete the end of this sentence until it becomes "You ha".');
    for (var i = 0; i < 12; ++i)
        evalAndLog('testRunner.execCommand("DeleteBackward")');
        setTimeout(editSentenceAsyncAndCheckMarker, timeoutToApplyPendingProgramaticSelectionChanges);
    } else
        shouldBecomeEqual('internals.hasGrammarMarker(4, 2)', 'false', finishJSTest);
}

var target = document.getElementById('src');

evalAndLog('document.execCommand("InsertText", false, "You has the right.")');

if (window.internals)
    shouldBecomeEqual('internals.hasGrammarMarker(4, 3)', 'true', function() {
        debug('Delete the end of this sentence until it becomes "You ha".')
        editSentenceAsyncAndCheckMarker();
    });
</script>
<script src="../../resources/js-test-post.js"></script>
</body>

Lines 1-3 a/LayoutTests/editing/spelling/resources/util.js_sec1
- a/LayoutTests/editing/spelling/resources/util.js +2 lines
		1	const timeoutToApplyPendingProgramaticSelectionChanges = 50;
		2
1	function log(msg)	3	function log(msg)
2	{	4	{
3	document.getElementById("console").innerHTML += (msg + "\n");	5	document.getElementById("console").innerHTML += (msg + "\n");

Lines 45-51 function testTwoLinesMisspellings() a/LayoutTests/editing/spelling/script-tests/spelling-backspace-between-lines.js_sec1
- a/LayoutTests/editing/spelling/script-tests/spelling-backspace-between-lines.js -1 / +1 lines
45	shouldBecomeEqual("internals.hasSpellingMarker(3, 2)", "true", done);	45	shouldBecomeEqual("internals.hasSpellingMarker(3, 2)", "true", done);
46	else	46	else
47	done();	47	done();
48	}, 100);	48	}, timeoutToApplyPendingProgramaticSelectionChanges);
49	}	49	}
50		50
51	function testMisspellingsAfterLineMergeUsingDelete()	51	function testMisspellingsAfterLineMergeUsingDelete()

Lines 59-65 function testMarkerForMisspelledWord(parentId, shouldBeMisspelled, id, type, spe a/LayoutTests/editing/spelling/spellcheck-attribute.html_sec1
- a/LayoutTests/editing/spelling/spellcheck-attribute.html -1 / +1 lines
59	shouldBecomeEqual('internals.hasSpellingMarker(0, 2)', shouldBeMisspelled ? 'true' : 'false', done);	59	shouldBecomeEqual('internals.hasSpellingMarker(0, 2)', shouldBeMisspelled ? 'true' : 'false', done);
60	else	60	else
61	done();	61	done();
62	}, 10);	62	}, timeoutToApplyPendingProgramaticSelectionChanges);
63	}	63	}
64		64
65	function logMarkup(id, spellcheckValue, shouldCloseTag)	65	function logMarkup(id, spellcheckValue, shouldCloseTag)

Lines 3-8 a/LayoutTests/editing/spelling/spelling-backspace-between-lines.html_sec1
- a/LayoutTests/editing/spelling/spelling-backspace-between-lines.html +1 lines
3	<head>	3	<head>
4	<script src="../../resources/js-test-pre.js"></script>	4	<script src="../../resources/js-test-pre.js"></script>
5	<script src="resources/js-test-selection-shared.js"></script>	5	<script src="resources/js-test-selection-shared.js"></script>
		6	<script src="resources/util.js"></script>
6	</head>	7	</head>
7	<body>	8	<body>
8	<p id="description"></p>	9	<p id="description"></p>



  if (typeof _a != "string" || typeof _b != "string")
    debug("WARN: shouldBecomeEqual() expects string arguments");

  function condition() {
    var exception;
    var _av;
    try {

      return true;
    }
    return false;
  }
  setTimeout(_waitForCondition, 0, condition, completionHandler);
}

function shouldBecomeEqualToString(value, reference, completionHandler)

  if (typeof _a != "string" || typeof _b != "string")
    debug("WARN: shouldBecomeDifferent() expects string arguments");

  function condition() {
    var exception;
    var _av;
    try {

      return true;
    }
    return false;
  }
  setTimeout(_waitForCondition, 0, condition, completionHandler);
}

function shouldBeTrue(_a) { shouldBe(_a, "true"); }

Return to Bug 133939

Lines 289-295 function shouldBecomeEqual(_a, _b, completionHandler) a/LayoutTests/resources/js-test-pre.js_sec1
- a/LayoutTests/resources/js-test-pre.js -6 / +6 lines
289	if (typeof _a != "string" \|\| typeof _b != "string")	289	if (typeof _a != "string" \|\| typeof _b != "string")
290	debug("WARN: shouldBecomeEqual() expects string arguments");	290	debug("WARN: shouldBecomeEqual() expects string arguments");
291		291
292	var condition = function() {	292	function condition() {
293	var exception;	293	var exception;
294	var _av;	294	var _av;
295	try {	295	try {
Lines 305-312 function shouldBecomeEqual(_a, _b, completionHandler) a/LayoutTests/resources/js-test-pre.js_sec2
305	return true;	305	return true;
306	}	306	}
307	return false;	307	return false;
308	};	308	}
309	_waitForCondition(condition, completionHandler);	309	setTimeout(_waitForCondition, 0, condition, completionHandler);
310	}	310	}
311		311
312	function shouldBecomeEqualToString(value, reference, completionHandler)	312	function shouldBecomeEqualToString(value, reference, completionHandler)
Lines 401-407 function shouldBecomeDifferent(_a, _b, completionHandler) a/LayoutTests/resources/js-test-pre.js_sec3
401	if (typeof _a != "string" \|\| typeof _b != "string")	401	if (typeof _a != "string" \|\| typeof _b != "string")
402	debug("WARN: shouldBecomeDifferent() expects string arguments");	402	debug("WARN: shouldBecomeDifferent() expects string arguments");
403		403
404	var condition = function() {	404	function condition() {
405	var exception;	405	var exception;
406	var _av;	406	var _av;
407	try {	407	try {
Lines 417-424 function shouldBecomeDifferent(_a, _b, completionHandler) a/LayoutTests/resources/js-test-pre.js_sec4
417	return true;	417	return true;
418	}	418	}
419	return false;	419	return false;
420	};	420	}
421	_waitForCondition(condition, completionHandler);	421	setTimeout(_waitForCondition, 0, condition, completionHandler);
422	}	422	}
423		423
424	function shouldBeTrue(_a) { shouldBe(_a, "true"); }	424	function shouldBeTrue(_a) { shouldBe(_a, "true"); }

Lines 1-3 a/Source/WebCore/ChangeLog_sec1
- a/Source/WebCore/ChangeLog +18 lines
		1	2014-12-16 Grzegorz Czajkowski <g.czajkowski@gmail.com>
		2
		3	shouldBecomeEqual() behaves as shouldBe() if the testing expression returns the expected value
		4	https://bugs.webkit.org/show_bug.cgi?id=133939
		5
		6	Reviewed by NOBODY (OOPS!).
		7
		8	Since programmatically selection change triggers spellchecker asynchronously
		9	call updateEditorUINowIfScheduled() in Internals::hasGrammarMarker() to apply
		10	pending document markers.
		11	Internals::hasSpellingMarker() already does it.
		12
		13	No new tests, covered by editing/spelling/grammar-edit-word.html
		14
		15	* testing/Internals.cpp:
		16	(WebCore::Internals::hasGrammarMarker):
		17	Call updateEditorUINowIfScheduled() to update document markers.
		18
1	2014-12-17 Zan Dobersek <zdobersek@igalia.com>	19	2014-12-17 Zan Dobersek <zdobersek@igalia.com>
2		20
3	[TexMap] Sprinkle range-based for-loops where still possible	21	[TexMap] Sprinkle range-based for-loops where still possible

Lines 1-3 a/LayoutTests/ChangeLog_sec1
- a/LayoutTests/ChangeLog +50 lines
		1	2014-12-15 Grzegorz Czajkowski <g.czajkowski@samsung.com>
		2
		3	shouldBecomeEqual() behaves as shouldBe() if the testing expression returns the expected value
		4	https://bugs.webkit.org/show_bug.cgi?id=133939
		5
		6	Reviewed by NOBODY (OOPS!).
		7
		8	First call of eval(expression) inside shouldBecomeEqual was always
		9	made synchronously. If the testing expression returns the same value
		10	as expected one then shouldBecomeEqual() will immediately report PASS,
		11	for example,
		12
		13	shouldBecomeEqual(internals.hasSpellingMarker("wellcome"), false);
		14
		15	in consequence, assuming asynchronous path of spellchecking, spelling markers
		16	may appear after a while.
		17
		18	The bug was caused by checking a condition at the beginning of shouldBecomeEqual(),
		19	before calling a timer. As a result, queued asynchronous events doesn't effect
		20	this checking.
		21
		22	* editing/spelling/editing-multiple-words-with-markers.html:
		23	Since this tests if spelling markers do not appear after a selection change
		24	after r164401 we need to call updateEditorUINowIfScheduled(). Without it editor
		25	would report old position of markers. Additionally, add timeout to make things
		26	testable in the browser.
		27	Without proposed change in shouldBecomeEqual, this bug was not revealed.
		28
		29	* editing/spelling/script-tests/spelling-backspace-between-lines.js:
		30	Replace magic number 100 with newly defined value.
		31	* editing/spelling/spellcheck-attribute.html: Ditto.
		32	* editing/spelling/spelling-backspace-between-lines.html: Ditto.
		33
		34	* editing/spelling/grammar-edit-word.html: Ditto.
		35	The reason why updateEditorUINowIfScheduled() is not mandatory here is that
		36	the test uses editing commands which already call updateEditorUINowIfScheduled().
		37	However, timeout needs to be added as while grammar checking
		38	Editor::respondToChangedSelection() is invoked in some circumstances
		39	and causes asynchronous spellchecker updates.
		40	This is probably a new bug that was not revealed yet.
		41
		42	* editing/spelling/resources/util.js: Ditto.
		43	Introduce the timeout value for pending selection updates that are
		44	required by spellchecker.
		45
		46	* resources/js-test-pre.js:
		47	(shouldBecomeEqual):
		48	(shouldBecomeDifferent):
		49	Always check a condition on timer.
		50
1	2014-12-16 Shivakumar JM <shiva.jm@samsung.com>	51	2014-12-16 Shivakumar JM <shiva.jm@samsung.com>
2		52
3	text node should not be created, On setting document.title to the empty string.	53	text node should not be created, On setting document.title to the empty string.

Lines 5-10 a/LayoutTests/editing/spelling/grammar-edit-word.html_sec1
- a/LayoutTests/editing/spelling/grammar-edit-word.html -7 / +10 lines
5	</head>	5	</head>
6	<body>	6	<body>
7	<script src="../../resources/js-test-pre.js"></script>	7	<script src="../../resources/js-test-pre.js"></script>
		8	<script src="resources/util.js"></script>
8	<div id="src" contenteditable="true" spellcheck="true"></div><br/>	9	<div id="src" contenteditable="true" spellcheck="true"></div><br/>
9	<script language="javascript">	10	<script language="javascript">
10	description('Test if WebKit removes grammar markers when we edit a grammatically-incorrect word. '	11	description('Test if WebKit removes grammar markers when we edit a grammatically-incorrect word. '
Lines 18-30 if (window.internals) { a/LayoutTests/editing/spelling/grammar-edit-word.html_sec2
18	internals.settings.setAsynchronousSpellCheckingEnabled(true);	19	internals.settings.setAsynchronousSpellCheckingEnabled(true);
19	}	20	}
20		21
21	function editSentenceAndCheckMarker()	22	function editSentenceAsyncAndCheckMarker() {
22	{	23	if (target.innerText != "You ha") {
23	debug('Delete the end of this sentence until it becomes "You ha".');
24	for (var i = 0; i < 12; ++i)
25	evalAndLog('testRunner.execCommand("DeleteBackward")');	24	evalAndLog('testRunner.execCommand("DeleteBackward")');
26		25	setTimeout(editSentenceAsyncAndCheckMarker, timeoutToApplyPendingProgramaticSelectionChanges);
27	shouldBecomeEqual('internals.hasGrammarMarker(4, 2)', 'false', finishJSTest);	26	} else
		27	shouldBecomeEqual('internals.hasGrammarMarker(4, 2)', 'false', finishJSTest);
28	}	28	}
29		29
30	var target = document.getElementById('src');	30	var target = document.getElementById('src');
Lines 32-38 target.focus(); a/LayoutTests/editing/spelling/grammar-edit-word.html_sec3
32	evalAndLog('document.execCommand("InsertText", false, "You has the right.")');	32	evalAndLog('document.execCommand("InsertText", false, "You has the right.")');
33		33
34	if (window.internals)	34	if (window.internals)
35	shouldBecomeEqual('internals.hasGrammarMarker(4, 3)', 'true', editSentenceAndCheckMarker);	35	shouldBecomeEqual('internals.hasGrammarMarker(4, 3)', 'true', function() {
		36	debug('Delete the end of this sentence until it becomes "You ha".')
		37	editSentenceAsyncAndCheckMarker();
		38	});
36	</script>	39	</script>
37	<script src="../../resources/js-test-post.js"></script>	40	<script src="../../resources/js-test-post.js"></script>
38	</body>	41	</body>