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 #777 is not readable, violating WCAG rules. While it's ok
to dim aliases and such, it must be at least #555 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
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
<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
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
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
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
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
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
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
"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
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
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
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
Two main mistakes:
* The {...foo} syntax is for a variable number of parameters.
But this is not the case here.
* Optional parameters should be marked as such via [foo].
Change-Id: I0c26ea44fab6094616443ce8fae4fd47c61fd7c4
This is covering parts of the TemplateDataHooks class. This
class does have a rather simple structure:
Either hook handlers are independent from each other. We don't
need to worry about accidental coverage then and can go with a
trivial top-level @covers tag.
Or some small helper methods are called. These are parts of
what's tested and should count as covered as well, I would
argue.
Change-Id: I6f419ae80b9ad78ff86ef2922db3178b29e244a4
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
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
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
… instead of 0. Conditionally add a dash in front as well to
avoid confusing results like '1' + sequence number = '12'.
Change-Id: I345704b00ba3812c4905f85e35cf21a6dfd05437
I looks like the Model.params data structure is build in a way
that it allows mismatching parameter "keys" and "names". E.g.
{
"a": { "name": "a" },
"a0": { "name": "a" },
}
There are comments in the code that suggest this is
intentional.
I found code that confused these two values and tries to use
the name as a key, for example. This fails, messes up the
paramOrder, and such.
This should not have much, if any effect for users because
users are blocked from doing this anyway, e.g. buttons get
disabled.
Change-Id: I2067024ad8d5b8e985a4f162cf6875f523777a6c
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
