Remove inadequate render-blocking styles for jquery.tablesorter
authorBartosz Dziewoński <matma.rex@gmail.com>
Mon, 28 May 2018 21:04:04 +0000 (23:04 +0200)
committerBartosz Dziewoński <matma.rex@gmail.com>
Mon, 28 May 2018 21:28:26 +0000 (23:28 +0200)
The current styles (from 1b14198df23ec26b44474e24621aeab276cbee5a)
did not actually apply to any elements on the page until
jquery.tablesorter ran: MediaWiki parser does not generate
`<thead>` elements and it's not even allowed as a HTML tag, only
jquery.tablesorter wraps a table header in them.

Instead, they resulted in the padding not being applied inside
VisualEditor editing surface (T195108), because it doesn't run
jquery.tablesorter (and instead manually adds CSS classes for
the sorting icons to appropriate cells).

The original attempt (from 8cdfcc5fd4ba36b7c91ac8097390220de230f8ae)
was a good idea, but I think it is not possible to do this well
enough with just CSS. In addition to unsortable columns (described
in T194451), the header may also consist of multiple rows, with cells
with colspans and rowspans, where only one header cell in each column
should have the sorting icon. This is not possible to implement in
CSS.

This reverts commit 1b14198df23ec26b44474e24621aeab276cbee5a
and parts of 8cdfcc5fd4ba36b7c91ac8097390220de230f8ae.

----

Minimal example of a table where it is impossible to style
appropriate header cells with CSS only:

  {| class="wikitable sortable"
  ! colspan="2" | H1-2
  ! rowspan="2" | H3
  |-
  ! H1 !! H2
  |-
  | A1 || A2 || A3
  |-
  | B1 || B2 || B3
  |}

Bug: T195108
Change-Id: Ife15069b3a2a38d36cb9077e31a82a9fc1a36215

includes/skins/Skin.php
resources/Resources.php
resources/src/jquery.tablesorter/jquery.tablesorter.less
resources/src/jquery/jquery.tablesorter.styles.less [deleted file]

index 6739c08..5c21bcd 100644 (file)
@@ -220,7 +220,6 @@ abstract class Skin extends ContextSource {
                // Preload jquery.tablesorter for mediawiki.page.ready
                if ( strpos( $out->getHTML(), 'sortable' ) !== false ) {
                        $modules['content'][] = 'jquery.tablesorter';
-                       $modules['styles']['content'][] = 'jquery.tablesorter.styles';
                }
 
                // Preload jquery.makeCollapsible for mediawiki.page.ready
index 4e0a452..42a3b07 100644 (file)
@@ -317,17 +317,10 @@ return [
                'styles' => 'resources/src/jquery.tablesorter/jquery.tablesorter.less',
                'messages' => [ 'sort-descending', 'sort-ascending' ],
                'dependencies' => [
-                       'jquery.tablesorter.styles',
                        'mediawiki.RegExp',
                        'mediawiki.language.months',
                ],
        ],
-       'jquery.tablesorter.styles' => [
-               'targets' => [ 'desktop', 'mobile' ],
-               'styles' => [
-                       'resources/src/jquery/jquery.tablesorter.styles.less',
-               ],
-       ],
        'jquery.textSelection' => [
                'scripts' => 'resources/src/jquery/jquery.textSelection.js',
                'dependencies' => 'jquery.client',
index 3bea471..ce24b0d 100644 (file)
@@ -8,10 +8,7 @@ table.jquery-tablesorter {
                cursor: pointer;
                background-repeat: no-repeat;
                background-position: center right;
-               // Note: To avoid reflows, a padding is set in
-               // the jquery.tableSorter.styles module as a render blocking style.
-               // Please do not add any CSS rules here that impact the positioning of the element
-               // e.g. padding, margin, position or float.
+               padding-right: 21px;
        }
 
        th.headerSortUp {
diff --git a/resources/src/jquery/jquery.tablesorter.styles.less b/resources/src/jquery/jquery.tablesorter.styles.less
deleted file mode 100644 (file)
index eda939d..0000000
+++ /dev/null
@@ -1,6 +0,0 @@
-.client-js {
-       // Reserve space for table sortable controls
-       table.sortable > thead > tr > th:not( .unsortable ) {
-               padding-right: 21px;
-       }
-}