From: Bartosz Dziewoński <matma.rex@gmail.com>
Date: Mon, 28 May 2018 21:04:04 +0000 (+0200)
Subject: Remove inadequate render-blocking styles for jquery.tablesorter
X-Git-Tag: 1.34.0-rc.0~5031^2
X-Git-Url: https://git.heureux-cyclage.org/?p=lhc%2Fweb%2Fwiklou.git;a=commitdiff_plain;h=e1940af9b63f127ef3fa7ef4984d6db44ece1c38

Remove inadequate render-blocking styles for jquery.tablesorter

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
---

diff --git a/includes/skins/Skin.php b/includes/skins/Skin.php
index 6739c08f8f..5c21bcd9d9 100644
--- a/includes/skins/Skin.php
+++ b/includes/skins/Skin.php
@@ -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
diff --git a/resources/Resources.php b/resources/Resources.php
index 4e0a452776..42a3b07875 100644
--- a/resources/Resources.php
+++ b/resources/Resources.php
@@ -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',
diff --git a/resources/src/jquery.tablesorter/jquery.tablesorter.less b/resources/src/jquery.tablesorter/jquery.tablesorter.less
index 3bea471f14..ce24b0de65 100644
--- a/resources/src/jquery.tablesorter/jquery.tablesorter.less
+++ b/resources/src/jquery.tablesorter/jquery.tablesorter.less
@@ -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
index eda939d770..0000000000
--- a/resources/src/jquery/jquery.tablesorter.styles.less
+++ /dev/null
@@ -1,6 +0,0 @@
-.client-js {
-	// Reserve space for table sortable controls
-	table.sortable > thead > tr > th:not( .unsortable ) {
-		padding-right: 21px;
-	}
-}