r94681 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r94680‎ | r94681 | r94682 >
Date:19:34, 16 August 2011
Author:brion
Status:ok
Tags:
Comment:
Followup r94609: fix jquery.highlightText test cases on IE, simplify test case setup so new ones can be added more trivially, uncommented the failing test case.

These tests were failing on IE 6, 7, and 8 because the innerHTML rendering of the highlighted text came out in caps and without quotes, such that the string didn't match the provided one. By re-normalizing the expected HTML form this check now works on those browsers. Switching common code to a loop and an array makes it easier to add new test cases: less duplication of code, and no need to manually update the number of test cases.

Note that the third test case was previously commented out, but looks like a serious regression. No longer commented out, so this will fail!
Modified paths:
  • /trunk/phase3/tests/qunit/suites/resources/jquery/jquery.highlightText.test.js (modified) (history)

Diff [purge]

Index: trunk/phase3/tests/qunit/suites/resources/jquery/jquery.highlightText.test.js
@@ -6,32 +6,34 @@
77 } );
88
99 test( 'Check', function() {
10 - expect(2);
 10+ var cases = [
 11+ {
 12+ text: 'Blue Öyster Cult',
 13+ highlight: 'Blue',
 14+ expected: '<span class="highlight">Blue</span> Öyster Cult'
 15+ },
 16+ {
 17+ text: 'Österreich',
 18+ highlight: 'Österreich',
 19+ expected: '<span class="highlight">Österreich</span>'
 20+ },
 21+ {
 22+ desc: 'Highlighter broken on punctuation mark',
 23+ text: 'So good. To be there',
 24+ highlight: 'good',
 25+ expected: 'So <span class="highlight">good</span>. To be there'
 26+ }
 27+ ];
 28+ expect(cases.length);
1129 var $fixture;
1230
13 - $fixture = $( '<p>Blue Öyster Cult</p>' );
14 - $fixture.highlightText( 'Blue' );
15 - equal(
16 - '<span class="highlight">Blue</span> Öyster Cult',
17 - $fixture.html()
18 - );
19 -
20 - $fixture = $( '<p>Österreich</p>' );
21 - $fixture.highlightText( 'Österreich' );
22 - equal(
23 - '<span class="highlight">Österreich</span>',
24 - $fixture.html()
25 - );
26 -
27 - /**
28 - * Highlighter broken on punctuation mark.
29 - */
30 - /** dont forget to update the value in expect() at the top!
31 - $fixture = $( '<p>So good. To be there</p>' );
32 - $fixture.highlightText( 'good' );
33 - equal(
34 - 'So <span class="highlight">good</span>. To be there',
35 - $fixture.html()
36 - );
37 - */
 31+ $.each(cases, function( i, item ) {
 32+ $fixture = $( '<p></p>' ).text( item.text );
 33+ $fixture.highlightText( item.highlight );
 34+ equals(
 35+ $fixture.html(),
 36+ $('<p>' + item.expected + '</p>').html(), // re-parse to normalize!
 37+ item.desc || undefined
 38+ );
 39+ } );
3840 } );

Follow-up revisions

RevisionCommit summaryAuthorDate
r94702Fix a regression in search highlighting from r90092...brion22:42, 16 August 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r94609tests for jquery.highlightText...hashar08:04, 16 August 2011

Status & tagging log