Commit graph

59 commits

Author SHA1 Message Date
Thiemo Kreuz 8497d85a2d Fix CSS styling of the HTML rendering broken since 2016
Some time ago there was a little bit of custom CSS applied to the HTML
table rendering. This is broken since patch I74214ea from 2016. This
patch renamed all CSS classes but forgot to update the PHP code
accordingly.

I decided to not change the HTML rendering because these class names
might already be used in custom per-wiki or per-user CSS. Instead I
partly revert I74214ea.

Unfortunately, some of the styles are quite dramatic, don't look good
or just don't work. I decided to remove some. The argument is that
the HTML rendering looks the same for 6 years now. I don't see a good
reason to change it now.

In detail:
* Suggested values are not aliases and should not be rendered in
  gray.
* The message "no description" is rendered in gray and italics. But
  this was applied to the wrong DOM element and made everything else
  gray and italic as well.
* The color  is not readable, violating WCAG rules. While it's ok
  to dim aliases and such, it must be at least  or darker.
* The "nowrap" destroys the table the moment one of the parameters
  does have a longer name or alias. Let the browser handle this, as
  it did for 6 years now.
* Same for rendering aliases as individual, indented blocks. This
  makes the table unnecessarily big when there are many aliases, and
  just doesn't look right. Again, let's stick to what we had for
  6 years.

Change-Id: Idfa76eed6e2d68474c79d4674efce091cb031b66
2022-08-08 11:56:16 +02:00
Thiemo Kreuz e297e767f0 Streamline HTML rendering code for format messages
The idea is to make it a little easier to follow what's going on
here.

This also improves an error message when tests fail.

Change-Id: If35be8aefab5a1568d53a9ecdc4313a66f71317b
2022-05-16 18:04:27 +00:00
Thiemo Kreuz d3f00177ba Don't extract template parameters from <pre>
<pre> behaves very similar to <nowiki> in so far that whatever it
contains doesn't get parsed as wikitext. It might contain {{{…}}}
tripple brackets, but these aren't going to be activated as template
parameters.

Bug: T91326
Change-Id: I05c24e369d97c48161c565e2ef30969ec28c6a23
2022-03-03 21:20:40 +00:00
jenkins-bot 67d6b77a84 Merge "Move getRawParams helper method to ApiTemplateData class" 2022-02-10 05:44:14 +00:00
Thiemo Kreuz da8969812c Move getRawParams helper method to ApiTemplateData class
This method is not used anywhere else:
https://codesearch.wmcloud.org/search/?q=getRawParams

I tried to make the code a bit more readable. Notable:
* Make use of the return value we get from the preg_… function.
* {{3,} means "the character '{' 3 or more times". {{{+ does the
  same. Note the { doesn't need to be escaped when it's not
  followed by a number.
* At the end, it doesn't make any difference when we scan for
  optional closing brackets. The moment we find at least 3 we are
  done.

The test is intentionally not moved. This is something for a later
patch.

Bug: T301337
Change-Id: I55e31ceecea2ae7c35bcfbc2d641b35f751820db
2022-02-09 12:37:33 +00:00
Thiemo Kreuz b1a4a27531 Split and streamline HTML formatter code
Bug: T301337
Change-Id: Ic2dac65e6e54411abc33326abf8d0fab148cb784
2022-02-09 12:37:29 +00:00
jenkins-bot 5407262418 Merge "Improve test coverage for HTML formatter code" 2022-02-08 16:09:17 +00:00
jenkins-bot a64ed0a376 Merge "Fix/narrow visibility of several methods" 2022-02-08 15:54:22 +00:00
Thiemo Kreuz a81a8b52bc Improve test coverage for HTML formatter code
… as well as some smaller improvements to the test coverage of other
code.

Change-Id: I5bec9c456fdc597c340dc0b515d23d837a7c5651
2022-02-03 12:45:27 +01:00
Thiemo Kreuz b674294789 Fix/narrow visibility of several methods
One method is only public to be able to test it. Others look like
they have been made "protected by default", which is not needed
anywhere.

Change-Id: Ib2231f0b2a879323aa53f8d40a175527c5b131d8
2022-02-03 10:48:04 +01:00
Thiemo Kreuz 001494f443 Move last remaining HTML formating code out of blob class
Effectively a no-op. This patch doesn't change what the code does.
Tests are in place to prove this.

As before, the tests are intentionally not moved but left in place.
This is for later patches to clean up.

Change-Id: If130e0d006a36d8c755288f8a4e4e9a4c42a6295
2022-02-03 09:33:03 +01:00
Thiemo Kreuz 8c24751491 Split validation and HTML formatting into separate services
No functional change was made to the code. It was only moved from one
place to another. Note there are a lot of tests that cover this code.
The tests haven't been touched on purpose. Splitting these as well
is something for a later patch.

Bug: T260980
Change-Id: I9fa0fa87768f2560b83a1b5f3d39211ea9d6cfad
2022-02-01 15:47:52 +01:00
Thiemo Kreuz 5f749c6418 Allow aliases to be integers in addition to strings
Parameter names in a template can be numeric. While it makes a lot of
sense to force a specific format in the TemplateData JSON (i.e. only
strings), it's inconvenient and confusing if numbers are rejected for
being "invalid".

Effects of this patch:
* The incoming JSON is allowed to contain numbers in the aliases
  array.
* However, the API normalizes these and forces all aliases to be
  strings, as it was always documented.
* The editor component accepts anything in the aliases array, but
  forces all aliases to be strings. Again, as documented.
* Note that it was never possible to use numeric keys in the `params`
  list. This patch is only about aliases.

At the moment this is a somewhat "hidden" feature. We might or might
not update the documentation to officially allow numeric aliases.

Bug: T298795
Change-Id: I32ea296b4520e7f21b03a1f6390db4f43b613bdd
2022-01-10 13:33:27 +01:00
Reedy 166812da07 Namespace extension
Change-Id: I779e97e512ec0c4f74fb6a4b706772fb1428e40f
2021-11-25 22:53:34 +00:00
jenkins-bot da2077d5bc Merge "Fix parameter auto-detection picking up syntax elements" 2021-10-06 23:27:08 +00:00
jenkins-bot 1c67ed5f97 Merge "Remove static keyword from all test code" 2021-10-06 23:27:06 +00:00
jenkins-bot f16838ebc0 Merge "Remove small pieces of unused code" 2021-10-06 23:05:34 +00:00
Thiemo Kreuz 2a9b7be921 Remove static keyword from all test code
I'm not sure why it was done this way. It doesn't need to.

Change-Id: Ie33ead5a3b6bddc464dd47e7e3153d6b8269b4c1
2021-10-02 10:43:07 +02:00
jenkins-bot 75ca493b68 Merge "Add test cases for (almost) all possible parsing errors" 2021-09-20 13:11:09 +00:00
Thiemo Kreuz d253fa4e28 Merge code paths in assertTemplateData() helper method
This streamlines the code of the helper method a bit, mostly by
avoiding duplication.

What actually happens is the exact same as before, with one
exception: When a test case doesn't have an expected "output",
the default (mostly empty) output does not run through the
roundtrip test. While doing this is not wrong, it doesn't tell
us anything about the specific test case.

Change-Id: I4a3d8a22c3dd6a9c5c3766195e5aef3cf37a6441
2021-09-06 08:48:01 +02:00
jenkins-bot 34503ed210 Merge "Use more strict assertSame() when comparing strings" 2021-09-04 05:19:49 +00:00
Thiemo Kreuz 3060559d1d Fix parameter auto-detection picking up syntax elements
See T290322 for a detailed description.

Bug: T290322
Change-Id: Id9935482fb466e7a1f6e55f042b13fe5851412d0
2021-09-03 13:18:42 +02:00
Thiemo Kreuz a7e1d60c64 Revert some unnecessary en→qqx changes
This reverts parts of I6c6342c.

Change-Id: I6e0e60604e393ffde21c3528ff3a26bfc01e66a2
2021-08-31 15:26:52 +02:00
Thiemo Kreuz eb12e48b14 Add test cases for (almost) all possible parsing errors
"Almost" because I found at least one that appears to be
unreachable (the very first check for null). But changing this
code is out of scope of this patch.

This also updates some of the error messages to explain the
location of the error better. It appears like the incomplete
paths are copy-paste mistakes.

I also found one duplicate test case and removed it.

Change-Id: Ic0ee9d04f5cd1060ade385ef308e70d221dd2f18
2021-08-30 15:17:21 +00:00
Thiemo Kreuz 696e3ed98f Make all tests use dummy language qqx instead of en
I find this good practice. It makes the tests more robust (e.g.
changes to a text don't make the test fail) and is potentially
faster, as no localization needs to be loaded.

Change-Id: I6c6342c80a40ab7260c35e7f1e3052aa4a9b9358
2021-08-30 17:15:41 +02:00
Thiemo Kreuz 2232a44638 Dramatically improve performance of random string generator
This test was reported as being slow (approx. 0.1s, but still).
This new implementation is 10 times faster, while still
fulfilling the requirements. While the new algorithm is more
predictable (every chunk is guaranteed to contain every
character exactly onece), it's obviously still good enough.

Neither the exact length of the generated string nor the exact
length of the gzipped string matter. PHP's random number
generator might be different – possibly generating a string
that compresses different. Newer versions of the gzip library
possibly save an extra byte. Who knows. This test shouldn't
care, as long as the gzipped string is long enough.

Compatibility with PHP 7.1 can be dropped as it is not
supported any more since MediaWiki 1.34, as far as I can tell.

Change-Id: I8d63390c9f4baa6084f932fa34068f606696cafc
2021-08-29 14:21:52 +02:00
Thiemo Kreuz 4766216948 Remove small pieces of unused code
Mostly unused variable initializations. Note I'm inlining some
`var` keywords in this patch. This is in line with the current
style guides. See for example the discussion in I4f198e2 (search
for "hoisted" in the comments). However, I'm not changing the
entire codebase, as this is not the goal of this patch and also
just not necessary at this point.

Change-Id: Ibd80566c44584851ee2530d6b16dd28eb3db6bfe
2021-08-28 12:10:22 +02:00
Thiemo Kreuz 930edf2419 Use more strict assertSame() when comparing strings
assertEquals() does have weird effects, like not reporting a
method that is expected to return null but returns an empty
string, and such. While it's usually not a problem, I learned
to avoid it.

Change-Id: I4f27ed5b200278021e051f1ab4d272f48e0bf344
2021-08-28 11:08:24 +02:00
jenkins-bot b745f0961a Merge "Make parameter order when sorting by status independent of language" 2021-08-02 12:51:37 +00:00
Thiemo Kreuz c2508a78ad Make parameter order when sorting by status independent of language
At the moment, when the user clicks the "Status" column to
sort by status, the statuses are ordered alphabetically,
which gives widely varying results depending on the language.
But there is an inherent order for these, even hard-coded in
the code: When a parameter is deprecated, nothing else matters.
Otherwise it's required → suggested → optional. Doesn't it
make much more sense to order the column this way? Especially
because there are never more than these 4 hard-coded values.

This is one of the (few remaining) issues mentioned on
https://de.wikipedia.org/wiki/Vorlage:TemplateData#Vorlagendokumentationsseite_verbessern_%E2%80%93_MediaWiki_ungen%C3%BCgend

This patch also makes it so that a CSS class name is always
added to all status fields, not only to the required ones.
This allows for per-wiki or per-user styling.

Change-Id: Id3f1ffafe09a3817972a4ee4bd4a3ded7be6f039
2021-07-30 13:42:00 +00:00
jenkins-bot 654e1ba037 Merge "Render docs with HTML5 <section>/<header> markup" 2021-07-30 13:40:14 +00:00
libraryupgrader a09e939fe7 build: Updating dependencies
composer:
* mediawiki/mediawiki-codesniffer: 36.0.0 → 37.0.0

npm:
* postcss: 7.0.35 → 7.0.36
  * https://npmjs.com/advisories/1693 (CVE-2021-23368)
* glob-parent: 5.1.0 → 5.1.2
  * https://npmjs.com/advisories/1751 (CVE-2020-28469)
* trim-newlines: 3.0.0 → 3.0.1
  * https://npmjs.com/advisories/1753 (CVE-2021-33623)

Change-Id: I23a441a089501a97f329b7b6d37bc658481e682f
2021-07-24 02:43:18 +00:00
Thiemo Kreuz 4b110c6c85 Render docs with HTML5 <section>/<header> markup
Change-Id: I91083561f7c4bcd3ee7184aee114a735de153fc4
2021-07-12 12:13:34 +02:00
Adam Wight 7b32bcefb4 Add suggested values parameter
Parameters may include a `suggestedvalues` property, which is rendered
in the UI for some parameter types.

TemplateData editor UI elements are implemented behind the
TemplateDataSuggestedValuesEditor feature flag.

Bug: T271897
Change-Id: I14012c79b3fa0d48c58fd8999584cc03ec03575e
2021-04-09 12:05:37 +02:00
jenkins-bot f26a5d37ec Merge "More strict validation of InterfaceTexts" 2021-02-13 17:23:44 +00:00
jenkins-bot 8b9c2f8b13 Merge "Remove inconsistent capitalization of parameter names" 2021-01-25 16:26:17 +00:00
Umherirrender b97d851664 Add missing @param to test documentation
Change-Id: Ie99e77e7e6f543eaf041eb8530c6f318538c8da5
2021-01-17 20:17:57 +01:00
Thiemo Kreuz 294786223d Remove inconsistent capitalization of parameter names
Example:

On
https://en.wikipedia.beta.wmflabs.org/wiki/Template:Anschutz
the two parameters "state" and "capitalization_test" don't have
a label in the <templatedata> JSON structure. Instead the
internal parameter name is shown. But it's capitalized for an
unknown reason. I guess this is done to make the table look
"nice". But it causes confusion – see the ticket.

This capitalization is there since the very first commits from
2013, see I16d3f9e.

Compare with VisualEditor: Edit the template on
https://en.wikipedia.beta.wmflabs.org/wiki/Conflict-title-0.8542952978413387-I%C3%B1t%C3%ABrn%C3%A2ti%C3%B4n%C3%A0liz%C3%A6ti%C3%B8n
or use the old wikitext editor and TemplateWizard to insert
the template. In both cases the parameter names are not
capitalized.

Another argument why this capitalization is misplaced: When
there is no <templatedata> JSON blob, the TemplateData editor
auto-detects the parameters and semi-automatically creates a
minimal JSON blob. This is the moment where labels should be
created and stored so the user can edit them. But this doesn't
happen (for good reasons).

The user can't do anything about the current capitalization.
The only way to change it is to add a label that does nothing
but repeat the parameter name, just to undo the capitalization.
That should not be the way this works, I would like to argue.

Bug: T174771
Change-Id: Ia8133d3f0d6b79fe89c63bb0392a334c0a185a65
2020-12-23 11:59:12 +00:00
Thiemo Kreuz c36cdcf862 Fix @covers tags in TemplateDataBlobTest
There is gzip-related code in this test, i.e. it is testing
the …Compressed… subclass as well.

Change-Id: I26c011be318427143a7d527f9f51f5c32c1e4fd3
2020-10-08 11:12:13 +02:00
Thiemo Kreuz f6de5d3493 Fix parameter extraction from wikitext source
This fixes a series of issues:
* The JS implementation had a trim() in one place that was
  missing in PHP.
* The actual parameter name in the paramNames/$params array was
  trimmed, but the "normalized" name (this is only for duplicate
  detection) was not trimmed.
* It was possible for an empty parameter to show up.

This resulted in very strange behavior, e.g. {{{ 1}}}{{{1 }}}
was detected as "1" and "10" (?), i.e. it would try to renumber
the duplicate in a strange way (string "1" plus a counter that
starts with 0).

Change-Id: I0a6371f3633b03b5b21809ecd06ea4c72d7d914d
2020-09-22 18:27:23 +02:00
Thiemo Kreuz c355a8fac4 More strict validation of InterfaceTexts
This fixes parts of a TODO in the code. Specifically:
* When an InterfaceText is an object, it can not be any object,
  but must be an stdClass.
* It can't be empty.
* Language codes must be non-empty strings.
* Values must be strings.

I'm intentionally not adding more validation for the language
codes, as this needs discussion (what values should be allowed?),
and can potentially break existing pages.

Same for empty strings. This can easily happen when users
manually create a <templatedata> tag and copy-paste pieces that
are meant to be filled in later. Empty strings are not really
invalid (again, this needs discussion), but might be something a
client want's to ignore.

Change-Id: I0facaa08cffe5a5a038423a58d55bc90a40b2d75
2020-08-19 15:29:55 +02:00
Siddharth VP 512911b92e Make getRawParams() ignore parameters in comments and nowiki tags
This is used by the includeMissingTitles option of API action=templatedata.
Parameter syntax existing within nowiki tags or comments will not be valid
for the template.

Bug: T237195
Change-Id: Ibbfa3e21488f2a37fc494862e929baf50607d4c9
2020-08-17 00:33:06 +00:00
adham-khatean 55a423a5ba Remove Sortable class from tables that have one row or less
Removing the sorting arrows from tables that have one row or less in
templates. By using variable $sorting that changes according to the
coung of elements in the array $data->params, to determine whether the
table should have "sortable" class or not.

Bug: T126150
Change-Id: I414c2375d4eb4da5d78f92f6b4e99b55e314ce4d
2020-06-22 09:13:17 +01:00
James D. Forrester c88aed986c build: Upgrade mediawiki-codesniffer to v28.0.0
Change-Id: I39190ff2056c24184f128eba71d619fc748b0ca5
2019-10-11 12:04:42 -07:00
saper ce13175017 Do not always store template data compressed
Only MySQL can handle blobs in page_props.pp_value

Bug: T203850
Change-Id: Ie20d412d89855e451d48f7656d97fb305b1c4c55
2019-09-30 14:24:39 +02:00
Timo Tijhof 4aaa9dfcf8 Remove default value for 'paramOrder'
When normalising a TemplateData blob for API consumers, we previously
automatically generated the 'paramOrder' with the order of the keys
as they were specified in the JSON blob (which, unlike in JS, is
known to be reliable in PHP).

While this was useful to some extent, it made it mandatory for
Parsoid and VisualEditor to always re-order properties during
edits to match the specified order.

In order to allow the order to remain flexible/unspecified, the
original Specification made paramOrder optional, but during the
implementation I gave it a default, which kind of defeated that
intention. This patch fixes that.

Bug: T138200
Change-Id: Ib40d23dac7e75274083f95a25c5aa1c22dfffb22
2019-01-08 19:55:59 +00:00
Timo Tijhof 498c66f451 tests: Add coverage for TemplateDataBlob::getHtml
Change-Id: I04caf657ac72f90d1dc0797f293d5a4f61cfdf08
2018-12-29 20:43:02 +00:00
Sam Wilson 75e54ae999 Ignore bang magic word in table syntax when guessing parameters
Three-braces-and-a-bang can be a table starting construct in templates.

This also fixes an overlooked bug in which the test wasn't checking
array element key names.

Bug: T157029
Change-Id: I69ed4fc9fe3bb126b7b39abea0f58ad56adf3885
2018-11-27 11:46:23 +08:00
Bartosz Dziewoński 9695c4fbb2 TemplateDataBlobTest: Ensure identical results on PHP 7.1
The implementation of mt_srand()/mt_rand() was changed in PHP 7.1 to
a better one. Pass MT_RAND_PHP to get the old implementation.
http://php.net/manual/en/function.mt-rand.php#refsect1-function.mt-rand-changelog

Bug: T206287
Change-Id: Ia2f79f2364583f7ee8f145519ae4ae1b7210c450
2018-10-05 11:08:01 -07:00
Sam Wilson 79e5347d90 Prevent leading hash from starting param names when guessing
Parameter names can be constructed with parser functions, so
we should not include these as raw parameter names. There
might be more characters to add here.

Bug: T203605
Change-Id: If57a9ed7edf1e881cd121d9a1bcb2e7455c04ec9
2018-09-12 10:04:45 +08:00