Parsoid added a class and, without it, we get selser complaining
about wrappers being modified, similar to T214649.
The "image" class is removed since Parsoid never added it (although it
now has "mw-file-description" for a similar purpose) and the legacy
parser doesn't apply it indiscriminately.
It doesn't seem like VE supports editing the |link= media option; it
just tries to roundtrip what's there and drops it on edit. The patch
here works with that limitation.
Galleries are found to drop href's, breaking selser, and should be fixed
in a follow up.
Bug: T292657
Bug: T303469
Change-Id: I92359048b42d32fe8a0f2cb79cd348cf5f2c56cc
It removes some indirection.
* Find: \$\( '<.+?' \)\.toArray\(\)
* Replace with: $.parseHTML( \1 )
(also replaced a few by hand that weren't caught by that regexp)
Change-Id: I2639cc4a26bc960046a189504dd8058344e14d60
These are only needed when we need to access a specific `this` from
within another `function () {}` context. This is not the case in the
situations here.
This is split from Ibf25d7e to make it smaller and easier to argue
about.
Change-Id: Ide1476de91fc343aa992ad92a1321d3a38b06dd0
It's not a getter, but a generator. I found the name confusing.
Getters typically don't return something different every time you
call them.
Change-Id: I6eeab8b6a8644e430003f6e1ad77ab4b28e0d8c9
When I type e.g. "subst:example" as the template name, we made this
work as the user would expect: the template named "Example" is found
and it's TemplateData documentation used. But the dialog title shows
"Subst:example". Note the uppercase "S". It means this string is
parsed as a title, including the "subst:". This is confusing. Just
show the template name.
Change-Id: I9817786991a8379cf48b0a664aef1413abddee2d
I was (again) wondering why the try-catch is there. But it's actually
needed. Now covered by a test.
Bug: T290140
Bug: T291062
Change-Id: Iccc7274ed9e7b81b8491dbca7d3d771706b0ed09
* The extra brackets are not needed in ES6.
* Make sure the name of the test is on the next line. This makes
the code easier to read.
Change-Id: Ib871dbfa27fcadc88e7335b9efb4d583bd4300ac
The code in .cacheTemplateDataApiResponse() where the `specCache`
is filled skips missing pages. .setTemplateData() is never called.
While we could – in theory – check the `missing` flag (as done in
patchset 1), this flag never makes it to the spec.
Rather simple solution: Mark everything as undocumented, as long
as .setTemplateData() is not called.
This affects only missing pages. .setTemplateData() is called in
all other situations.
Bug: T272487
Bug: T276574
Bug: T290136
Change-Id: I7045e84f2f2ba5aa4591c94ea495b0249e6c40d6
Not only do we want to make sure getUniquePartId() always starts
at 0 and increments correctly, it should return a number (and
not e.g. "part_0").
I realize the getTitle() test is also testing functionality from
mw.libs.ve.… (can be found in the file ve.utils.parsoid.js).
This is intentional. What we care about at this point is not a
library but the very specific functionality of a very specific
method we use quite a lot in code we touch.
Bug: T289560
Change-Id: I43c1d00dacf27a68b16f62ecca4adda22f437391
These tests obviously don't need this extra environment.
They run just fine (and faster) without.
Bug: T289560
Change-Id: Ib186a07cd556f741e0440ffa54ae6aaaf626adcd
This reverts commit 0d4dee341b.
Reason for revert: This made it entirely impossible to add a
deprecated parameter, even if done intentionally. Needs more tests.
Bug: T274551
Change-Id: I7389bad0845cd1ce78f9d7ef71592cb1ce2a063e
Separation of concerns:
* The template model knows which parameters are currently used,
but doesn't know what's documented.
* The spec knows what's documented, but doesn't know what's
currently used.
Change-Id: I97cac00d6775a17a07059d0e8a7a116adc6080b3
There are some methods that behave different when a parameter
placeholder (where the parameter name is "") is present. Some
skip placeholders, some don't. This is critical to cover
before we make further changes to this class.
What I also do in this patch:
* Use shorter variable names to make the code easier to read.
* Don't reuse the `transclusionData` variable but use a copy
of the expected value. This makes the assertions much
easier to understand.
* Bring every test in the same "setup" → "execute" → "assert"
order.
Change-Id: I41a691c56bc509b132dc719ff820ae1ade4ccc3a
A lot of the checks are redundant. The first check still is
redundant because the later two cover everything as well. But
I left it for performance reasons.
Additionally:
* There was no test for the method.
* This patch also updates a few pieces of documentation in the
same class.
Change-Id: I10f2944a844cc070bdc08dec6719929b383e34fa
If a known parameter is present using one of it's aliases, then
only the aliased name should be shown to the user. This patch,
therefore, resolves the issue of the same parameter being added
to the sidebar twice.
When adding a parameter that is aliased, it will receive the same
position as the non-aliased parameter it is replacing.
Bug: T274545
Change-Id: If4e58c941fd0f0e690d3603935f5a5d3f9938163
This fixes a minor issue in the spec class. In the first step,
parameters from the template are added to the list of known
parameters. Later, aliases are resolved. The original behavior
was that such a parameter moved to the end of the list. This
is rather unexpected.
This dosn't have much of an impact. The pretty much only place
where the parameter order from the spec can be seen is in the
parameter search widget. Still I believe it's worth fixing.
Bug: T285483
Change-Id: I455818451811e92bba3e9320c2d41e1db8d563f2
I don't want this code to crash when the TemplateData API
returns an unexpected result.
Bug: T285483
Change-Id: I237cbfbb85892a53a08d9e7e34cf4974775d627a
This doesn't "extend". It was never used like this. What it
actually does is to link between a (cached) TemplateData blob
and the spec class that want's to use it.
Is this the best possible name?
* fillFromTemplateData( … )?
* propagateTemplateDocumentation( … )?
* readDocumentationFrom( templateData )?
* …?
Do we want to rename the "spec" class as well?
* MWTemplateDocumentation?
* MWTemplateMetadata?
* MWTemplateDataAccessor?
* …?
Bug: T285483
Change-Id: I6c52ef42d411c2f47fc0080768d36ebda4dd2a55
The so called "spec" class keeps track of parameters that have
been used before, no matter if documented via TemplateData or
not. Removed parameters are still "known" (i.e. have been seen
before).
This feature allows to easily find previously used parameters
names when an undocumented parameter was removed and the user
tries to add it again.
Bug: T285483
Change-Id: Ia1555eea87cd99e7a3f386f4279ec5a80fb98a79
In I04b8a14fbec7be5a1c4defabf92e94f694c1e638 we sepearted params from
aliases. There we missed that re-filling the parameters from the
template could re-add the aliases.
Bug: T285483
Bug: T285843
Change-Id: I1928b443a5f708bc8c57efa5ad0a86b5915b159c
While the term "canonical" is not wrong, I find it still
somewhat ambiguous.
1. "Canonical" could mean different things. E.g. is the order
of parameters as they appear in the article's wikitext the
"canonical" one? It's possible to argue like this, esp. if a
template doesn't have TemplateData documentation. In this case
the only order known is the one from the wikitext.
2. "Canonical" sounds like the parameters must be reordered.
But this should never happen. Not having dirty diffs is more
important than having the parameters in a specific order.
Bug: T285483
Change-Id: I23658d37fea50b727667677ac6a49066673b2135
Wait, what's going on here? This patch looks like it changes the
behavior of this code. But it doesn't. Here is what happened
before:
* Let's say a template contains 2 parameters, A and B.
* We don't know yet if these names are aliases.
* getParameterNames() returns [ "A", "B" ].
* extend() is called. The TemplateData documentation contains
the parameters "B" and "C". "C" does have an alias "A".
* extend() can't find "C" and adds it to the end, as if it's a
new parameter.
* extend() also iterates the aliases. For each alias it creates
a reference to the specification object. In this case a
reference from "A" to "C" is created.
* But "A" already exists. The position of "A" doesn't change,
but the specification now says it's an alias.
* getParameterNames() skips aliases. It skips "A" and instead
returns the new "C" from the end of the list.
This was the behavior before. It's unchanged, proven by the tests.
Change-Id: I04b8a14fbec7be5a1c4defabf92e94f694c1e638
The idea is to not actually store all these default values, but
fall back to the default only when needed.
Some more details:
* The only remaining property is ….name. The only reason to
have this property is to distinguish between aliases and
primary parameter names. This will be reworked in a later
patch.
* The description falls back to null because this is the
documented fallback, not undefined.
* The default value falls back to "", same as the auto-value.
Why not null you might ask. This is intentional. Both the
auto- and default value are effectively wikitext snippets,
while the example is a label in the VE UI.
Bug: T285483
Change-Id: I1be3cca18f9ad6fc1c16362b24633f7613f02539
