r98053 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r98052‎ | r98053 | r98054 >
Date:02:58, 25 September 2011
Author:dantman
Status:ok (Comments)
Tags:
Comment:
Followup r94465; Add parser tests and turn the feature on-by-default like I intended for it (I guess I forgot to return it to true when I was testing it as off)
Modified paths:
  • /trunk/phase3/includes/DefaultSettings.php (modified) (history)
  • /trunk/phase3/tests/parser/parserTests.txt (modified) (history)

Diff [purge]

Index: trunk/phase3/tests/parser/parserTests.txt
@@ -1346,7 +1346,7 @@
13471347 !! test
13481348 Table rowspan
13491349 !! input
1350 -{| align=right border=1
 1350+{| border=1
13511351 | Cell 1, row 1
13521352 |rowspan=2| Cell 2, row 1 (and 2)
13531353 | Cell 3, row 1
@@ -1355,7 +1355,7 @@
13561356 | Cell 3, row 2
13571357 |}
13581358 !! result
1359 -<table align="right" border="1">
 1359+<table border="1">
13601360 <tr>
13611361 <td> Cell 1, row 1
13621362 </td>
@@ -1968,13 +1968,13 @@
19691969 !! test
19701970 Failing to transform badly formed HTML into correct XHTML
19711971 !! input
1972 -<br clear=left>
1973 -<br clear=right>
1974 -<br clear=all>
 1972+<br style="clear: left;">
 1973+<br style="clear: right;">
 1974+<br style="clear: both;">
19751975 !! result
1976 -<p><br clear="left" />
1977 -<br clear="right" />
1978 -<br clear="all" />
 1976+<p><br style="clear: left;" />
 1977+<br style="clear: right;" />
 1978+<br style="clear: both;" />
19791979 </p>
19801980 !!end
19811981
@@ -4517,9 +4517,9 @@
45184518 !! test
45194519 div with illegal double attributes
45204520 !! input
4521 -<div align="center" align="right">HTML rocks</div>
 4521+<div id="a" id="b">HTML rocks</div>
45224522 !! result
4523 -<div align="right">HTML rocks</div>
 4523+<div id="b">HTML rocks</div>
45244524
45254525 !!end
45264526
@@ -4549,9 +4549,9 @@
45504550 !! test
45514551 DIV IN UPPERCASE
45524552 !! input
4553 -<DIV ALIGN="center">HTML ROCKS</DIV>
 4553+<DIV ID="x">HTML ROCKS</DIV>
45544554 !! result
4555 -<div align="center">HTML ROCKS</div>
 4555+<div id="x">HTML ROCKS</div>
45564556
45574557 !!end
45584558
@@ -8799,6 +8799,22 @@
88008800 ...
88018801 !! end
88028802
 8803+!! test
 8804+Deprecated presentational attributes are converted to css
 8805+!! input
 8806+{|
 8807+| valign=top align=left width=100 height=25% | Asdf
 8808+|}
 8809+<ul type="disc"></ul>
 8810+!! result
 8811+<table>
 8812+<tr>
 8813+<td style="text-align: left; height: 25%; vertical-align: top; width: 100px;"> Asdf
 8814+</td></tr></table>
 8815+<ul style="list-style-type: disc;"></ul>
 8816+
 8817+!! end
 8818+
88038819 TODO:
88048820 more images
88058821 more tables
Index: trunk/phase3/includes/DefaultSettings.php
@@ -2186,7 +2186,7 @@
21872187 /**
21882188 * Cleanup as much presentational html like valign -> css vertical-align as we can
21892189 */
2190 -$wgCleanupPresentationalAttributes = false;
 2190+$wgCleanupPresentationalAttributes = true;
21912191
21922192 /**
21932193 * Should we try to make our HTML output well-formed XML? If set to false,

Follow-up revisions

RevisionCommit summaryAuthorDate
r98056Followup r98053; Add $wgCleanupPresentationalAttributes to the globals setup ...dantman04:35, 25 September 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r94465Add code to the sanitizer to convert presontational attributes that were remo...dantman16:41, 14 August 2011

Comments

#Comment by Krinkle (talk | contribs)   21:34, 25 September 2011
 !! test
 Table rowspan
 !! input
-{| align=right border=1
+{| border=1
 | Cell 1, row 1 
 |rowspan=2| Cell 2, row 1 (and 2) 
 | Cell 3, row 1 
@@ -1355,7 +1355,7 @@
 | Cell 3, row 2 
 |}
 !! result
-<table align="right" border="1">
+<table border="1">
 <tr>

Why is the border attribute removed from the test input ? Shouldn't just the result be changed to set float:right or something ? Either way, commit message says nothing about this. Marking fixme.

#Comment by Dantman (talk | contribs)   21:40, 25 September 2011

...the border attribute is still there.

I removed deprecated attributes from other tests so that changes in behaviour of the deprecated attribute cleaning code only affects the test actually testing for that, not causing other tests that aren't testing for that to break.

#Comment by Krinkle (talk | contribs)   21:43, 25 September 2011

Sorry, I meant the align attribute of course :)

But that is now being tested in a dedicated test. Thanks.

#Comment by Dantman (talk | contribs)   21:48, 25 September 2011

;) along with valign, width and height in both numeric and percentage types, and the list 'type' attribute.

#Comment by Hashar (talk | contribs)   11:05, 5 December 2011

Is there anything to do there or did Krinkle just forgot to mark this revision  OK ? :)

Status & tagging log