r78196 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r78195‎ | r78196 | r78197 >
Date:16:03, 10 December 2010
Author:demon
Status:ok (Comments)
Tags:
Comment:
Revert r68230, r68231 (mode="grid" on tables) per CR. It's in the history if anyone wants to work on this again
Modified paths:
  • /trunk/phase3/includes/Sanitizer.php (modified) (history)
  • /trunk/phase3/includes/parser/Parser.php (modified) (history)
  • /trunk/phase3/skins/common/shared.css (modified) (history)

Diff [purge]

Index: trunk/phase3/skins/common/shared.css
@@ -883,69 +883,7 @@
884884 a.sortheader {
885885 margin: 0 0.3em;
886886 }
887 -
888 -.grid-table {
889 - display: table;
890 -}
891 -
892 -.grid-row {
893 - display: table-row;
894 -}
895 -
896 -.grid-cell {
897 - display: table-cell;
898 - padding: 0.33ex;
899 -}
900 -
901 -/* Localised ordered list numbering for some languages */
902 -ol:lang(bcc) li,
903 -ol:lang(bqi) li,
904 -ol:lang(fa) li,
905 -ol:lang(glk) li,
906 -ol:lang(kk-arab) li,
907 -ol:lang(mzn) li {
908 - list-style-type: -moz-persian;
909 - list-style-type: persian;
910 -}
911 -
912 -ol:lang(ckb) li {
913 - list-style-type: -moz-arabic-indic;
914 - list-style-type: arabic-indic;
915 -}
916 -
917 -ol:lang(bn) li {
918 - list-style-type: -moz-bengali;
919 - list-style-type: bengali;
920 -}
921 -
922 -/* tooltip styles */
923 -.mw-help-field-hint {
924 - display: none;
925 - padding: 0px;
926 - padding-left: 15px;
927 - margin-left: 2px;
928 - margin-bottom: -8px;
929 - background-image: url('images/help-question.gif');
930 - background-position: left center;
931 - background-repeat: no-repeat;
932 - color: #0645ad;
933 - text-decoration: underline;
934 - cursor: pointer;
935 - font-size: .8em;
936 -}
937 -.mw-help-field-hint:hover {
938 - background-image: url('images/help-question-hover.gif');
939 -}
940 -.mw-help-field-data {
941 - display: block;
942 - background-color: #d6f3ff;
943 - padding:5px 8px 4px 8px;
944 - border: 1px solid #5dc9f4;
945 - margin-left: 20px;
946 -}
947887 .tipsy { padding: 5px 5px 10px; font-size: 12px; position: absolute; z-index: 100000; overflow: visible; }
948888 .tipsy-inner { padding: 5px 8px 4px 8px; background-color: #d6f3ff; color: black; border: 1px solid #5dc9f4; max-width: 300px; text-align: left; }
949889 .tipsy-arrow { position: absolute; background: url( 'images/tipsy-arrow.gif' ) no-repeat top left; width: 13px; height: 13px; }
950890 .tipsy-se .tipsy-arrow { bottom: -2px; right: 10px; background-position: 0% 100%; }
951 -
952 -
Index: trunk/phase3/includes/parser/Parser.php
@@ -797,18 +797,6 @@
798798 $has_opened_tr = array(); # Did this table open a <tr> element?
799799 $indent_level = 0; # indent level of the table
800800
801 - $table_tag = 'table';
802 - $tr_tag = 'tr';
803 - $th_tag = 'th';
804 - $td_tag = 'td';
805 - $caption_tag = 'caption';
806 -
807 - $extra_table_attribs = null;
808 - $extra_tr_attribs = null;
809 - $extra_td_attribs = null;
810 -
811 - $convert_style = false;
812 -
813801 foreach ( $lines as $outLine ) {
814802 $line = trim( $outLine );
815803
@@ -825,31 +813,9 @@
826814 $indent_level = strlen( $matches[1] );
827815
828816 $attributes = $this->mStripState->unstripBoth( $matches[2] );
 817+ $attributes = Sanitizer::fixTagAttributes( $attributes , 'table' );
829818
830 - $attr = Sanitizer::decodeTagAttributes( $attributes );
831 -
832 - $mode = @$attr['mode'];
833 - if ( !$mode ) $mode = 'data';
834 -
835 - if ( $mode == 'grid' || $mode == 'layout' ) {
836 - $table_tag = 'div';
837 - $tr_tag = 'div';
838 - $th_tag = 'div';
839 - $td_tag = 'div';
840 - $caption_tag = 'div';
841 -
842 - $extra_table_attribs = array( 'class' => 'grid-table' );
843 - $extra_tr_attribs = array( 'class' => 'grid-row' );
844 - $extra_td_attribs = array( 'class' => 'grid-cell' );
845 -
846 - $convert_style = true;
847 - }
848 -
849 - if ($convert_style) $attr['style'] = Sanitizer::styleFromAttributes( $attr );
850 - $attr = Sanitizer::validateTagAttributes( $attr, $table_tag );
851 - $attributes = Sanitizer::collapseTagAttributes( $attr, $extra_table_attribs );
852 -
853 - $outLine = str_repeat( '<dl><dd>' , $indent_level ) . "<$table_tag{$attributes}>";
 819+ $outLine = str_repeat( '<dl><dd>' , $indent_level ) . "<table{$attributes}>";
854820 array_push( $td_history , false );
855821 array_push( $last_tag_history , '' );
856822 array_push( $tr_history , false );
@@ -861,15 +827,15 @@
862828 continue;
863829 } elseif ( substr( $line , 0 , 2 ) === '|}' ) {
864830 # We are ending a table
865 - $line = "</$table_tag>" . substr( $line , 2 );
 831+ $line = '</table>' . substr( $line , 2 );
866832 $last_tag = array_pop( $last_tag_history );
867833
868834 if ( !array_pop( $has_opened_tr ) ) {
869 - $line = "<$tr_tag><$td_tag></$td_tag></$tr_tag>{$line}";
 835+ $line = "<tr><td></td></tr>{$line}";
870836 }
871837
872838 if ( array_pop( $tr_history ) ) {
873 - $line = "</$tr_tag>{$line}";
 839+ $line = "</tr>{$line}";
874840 }
875841
876842 if ( array_pop( $td_history ) ) {
@@ -883,12 +849,7 @@
884850
885851 # Whats after the tag is now only attributes
886852 $attributes = $this->mStripState->unstripBoth( $line );
887 -
888 - $attr = Sanitizer::decodeTagAttributes( $attributes );
889 - if ($convert_style) $attr['style'] = Sanitizer::styleFromAttributes( $attr );
890 - $attr = Sanitizer::validateTagAttributes( $attr, $tr_tag );
891 - $attributes = Sanitizer::collapseTagAttributes( $attr, $extra_tr_attribs );
892 -
 853+ $attributes = Sanitizer::fixTagAttributes( $attributes, 'tr' );
893854 array_pop( $tr_attributes );
894855 array_push( $tr_attributes, $attributes );
895856
@@ -898,7 +859,7 @@
899860 array_push( $has_opened_tr , true );
900861
901862 if ( array_pop( $tr_history ) ) {
902 - $line = "</$tr_tag>";
 863+ $line = '</tr>';
903864 }
904865
905866 if ( array_pop( $td_history ) ) {
@@ -936,7 +897,7 @@
937898 if ( $first_character !== '+' ) {
938899 $tr_after = array_pop( $tr_attributes );
939900 if ( !array_pop( $tr_history ) ) {
940 - $previous = "<$tr_tag{$tr_after}>\n";
 901+ $previous = "<tr{$tr_after}>\n";
941902 }
942903 array_push( $tr_history , true );
943904 array_push( $tr_attributes , '' );
@@ -951,11 +912,11 @@
952913 }
953914
954915 if ( $first_character === '|' ) {
955 - $last_tag = $td_tag;
 916+ $last_tag = 'td';
956917 } elseif ( $first_character === '!' ) {
957 - $last_tag = $th_tag;
 918+ $last_tag = 'th';
958919 } elseif ( $first_character === '+' ) {
959 - $last_tag = $caption_tag;
 920+ $last_tag = 'caption';
960921 } else {
961922 $last_tag = '';
962923 }
@@ -965,24 +926,15 @@
966927 # A cell could contain both parameters and data
967928 $cell_data = explode( '|' , $cell , 2 );
968929
969 - $attributes = '';
970 -
971930 # Bug 553: Note that a '|' inside an invalid link should not
972931 # be mistaken as delimiting cell parameters
973932 if ( strpos( $cell_data[0], '[[' ) !== false ) {
974 - if ($extra_td_attribs) $attributes = Sanitizer::collapseTagAttributes( $extra_td_attribs );
975 - $cell = "{$previous}<{$last_tag}{$attributes}>{$cell}";
 933+ $cell = "{$previous}<{$last_tag}>{$cell}";
976934 } elseif ( count( $cell_data ) == 1 ) {
977 - if ($extra_td_attribs) $attributes = Sanitizer::collapseTagAttributes( $extra_td_attribs );
978 - $cell = "{$previous}<{$last_tag}{$attributes}>{$cell_data[0]}";
 935+ $cell = "{$previous}<{$last_tag}>{$cell_data[0]}";
979936 } else {
980937 $attributes = $this->mStripState->unstripBoth( $cell_data[0] );
981 -
982 - $attr = Sanitizer::decodeTagAttributes( $attributes );
983 - if ($convert_style) $attr['style'] = Sanitizer::styleFromAttributes( $attr );
984 - $attr = Sanitizer::validateTagAttributes( $attr, $last_tag );
985 - $attributes = Sanitizer::collapseTagAttributes( $attr, $extra_td_attribs );
986 -
 938+ $attributes = Sanitizer::fixTagAttributes( $attributes , $last_tag );
987939 $cell = "{$previous}<{$last_tag}{$attributes}>{$cell_data[1]}";
988940 }
989941
@@ -996,16 +948,16 @@
997949 # Closing open td, tr && table
998950 while ( count( $td_history ) > 0 ) {
999951 if ( array_pop( $td_history ) ) {
1000 - $out .= "</$td_tag>\n";
 952+ $out .= "</td>\n";
1001953 }
1002954 if ( array_pop( $tr_history ) ) {
1003 - $out .= "</$tr_tag>\n";
 955+ $out .= "</tr>\n";
1004956 }
1005957 if ( !array_pop( $has_opened_tr ) ) {
1006 - $out .= "<$tr_tag><$td_tag></$td_tag></$tr_tag>\n" ;
 958+ $out .= "<tr><td></td></tr>\n" ;
1007959 }
1008960
1009 - $out .= "</$table_tag>\n";
 961+ $out .= "</table>\n";
1010962 }
1011963
1012964 # Remove trailing line-ending (b/c)
@@ -1014,7 +966,7 @@
1015967 }
1016968
1017969 # special case: don't return empty table
1018 - if ( $out === "<$table_tag>\n<$tr_tag><$td_tag></$td_tag></$tr_tag>\n</$table_tag>" ) {
 970+ if ( $out === "<table>\n<tr><td></td></tr>\n</table>" ) {
1019971 $out = '';
1020972 }
1021973
Index: trunk/phase3/includes/Sanitizer.php
@@ -796,51 +796,6 @@
797797 }
798798 }
799799
800 - /**
801 - * Take an associative array of attribute name/value pairs
802 - * and generate a css style representing all the style-related
803 - * attributes. If there already a style attribute in the array,
804 - * it is also included in the value returned.
805 - */
806 - static function styleFromAttributes( $attributes ) {
807 - $styles = array();
808 -
809 - foreach ( $attributes as $attribute => $value ) {
810 - if ( $attribute == 'bgcolor' ) {
811 - $styles[] = "background-color: $value";
812 - } else if ( $attribute == 'border' ) {
813 - $styles[] = "border-width: $value";
814 - } else if ( $attribute == 'align' ) {
815 - $styles[] = "text-align: $value";
816 - } else if ( $attribute == 'valign' ) {
817 - $styles[] = "vertical-align: $value";
818 - } else if ( $attribute == 'width' ) {
819 - if ( preg_match( '/\d+/', $value ) === false ) {
820 - $value .= 'px';
821 - }
822 -
823 - $styles[] = "width: $value";
824 - } else if ( $attribute == 'height' ) {
825 - if ( preg_match( '/\d+/', $value ) === false ) {
826 - $value .= 'px';
827 - }
828 -
829 - $styles[] = "height: $value";
830 - } else if ( $attribute == 'nowrap' ) {
831 - if ( $value ) {
832 - $styles[] = "white-space: nowrap";
833 - }
834 - }
835 - }
836 -
837 - if ( isset( $attributes[ 'style' ] ) ) {
838 - $styles[] = $attributes[ 'style' ];
839 - }
840 -
841 - if ( !$styles ) return '';
842 - else return implode( '; ', $styles );
843 - }
844 -
845800 /**
846801 * Take a tag soup fragment listing an HTML element's attributes
847802 * and normalize it to well-formed XML, discarding unwanted attributes.
@@ -858,66 +813,24 @@
859814 *
860815 * @param $text String
861816 * @param $element String
862 - * @param $defaults Array (optional) associative array of default attributes to splice in.
863 - * class and style attributes are combined. Otherwise, values from
864 - * $attributes take precedence over values from $defaults.
865817 * @return String
866818 */
867 - static function fixTagAttributes( $text, $element, $defaults = null ) {
 819+ static function fixTagAttributes( $text, $element ) {
868820 if( trim( $text ) == '' ) {
869821 return '';
870822 }
871823
872 - $decoded = Sanitizer::decodeTagAttributes( $text );
873 - $stripped = Sanitizer::validateTagAttributes( $decoded, $element );
874 - $attribs = Sanitizer::collapseTagAttributes( $stripped, $defaults );
 824+ $stripped = Sanitizer::validateTagAttributes(
 825+ Sanitizer::decodeTagAttributes( $text ), $element );
875826
876 - return $attribs;
877 - }
878 -
879 - /**
880 - * Take an associative array or attribute name/value pairs
881 - * and collapses it to well-formed XML.
882 - * Does not filter attributes.
883 - * Output is safe for further wikitext processing, with escaping of
884 - * values that could trigger problems.
885 - *
886 - * - Double-quotes all attribute values
887 - * - Prepends space if there are attributes.
888 - *
889 - * @param $attributes Array is an associative array of attribute name/value pairs.
890 - * Assumed to be sanitized already.
891 - * @param $defaults Array (optional) associative array of default attributes to splice in.
892 - * class and style attributes are combined. Otherwise, values from
893 - * $attributes take precedence over values from $defaults.
894 - * @return String
895 - */
896 - static function collapseTagAttributes( $attributes, $defaults = null ) {
897 - if ( $defaults ) {
898 - foreach( $defaults as $attribute => $value ) {
899 - if ( isset( $attributes[ $attribute ] ) ) {
900 - if ( $attribute == 'class' ) {
901 - $value .= ' '. $attributes[ $attribute ];
902 - } else if ( $attribute == 'style' ) {
903 - $value .= '; ' . $attributes[ $attribute ];
904 - } else {
905 - continue;
906 - }
907 - }
908 -
909 - $attributes[ $attribute ] = $value;
910 - }
911 - }
912 -
913 - $chunks = array();
914 -
915 - foreach( $attributes as $attribute => $value ) {
 827+ $attribs = array();
 828+ foreach( $stripped as $attribute => $value ) {
916829 $encAttribute = htmlspecialchars( $attribute );
917830 $encValue = Sanitizer::safeEncodeAttribute( $value );
918831
919 - $chunks[] = "$encAttribute=\"$encValue\"";
 832+ $attribs[] = "$encAttribute=\"$encValue\"";
920833 }
921 - return count( $chunks ) ? ' ' . implode( ' ', $chunks ) : '';
 834+ return count( $attribs ) ? ' ' . implode( ' ', $attribs ) : '';
922835 }
923836
924837 /**

Follow-up revisions

RevisionCommit summaryAuthorDate
r78298Fix r78196: readd accidentally removed lines in shared.csscatrope12:40, 13 December 2010

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r68230let mode="grid" on a wiki table trigger output as div soup instead of a html ...daniel19:17, 18 June 2010
r68231follop-up to 68230: forgot to commit the styles needed for layout gridsdaniel19:21, 18 June 2010

Comments

#Comment by Nikerabbit (talk | contribs)   20:05, 10 December 2010

Reverts too much:

-/* Localised ordered list numbering for some languages */
#Comment by Catrope (talk | contribs)   12:40, 13 December 2010

Re-added in r78298.

Status & tagging log