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
This is done for two reasons:
1. It fixes the behavior of two methods in rare edge-case
situations. They aren't documented to return undefined.
2. It reduces the amount of stuff this class stores when it's
nothing but a default value anyway. Note this patch does this
for the template-level properties only. Another patch will do
the same for the parameter-level properties.
Bug: T285483
Change-Id: If2e4d56da1fa52e32dc94191f36d7dc6a1487829
This reflects much better how this method is meant to behave.
Note I will continue to remove documentation that doesn't
explain anything in addition to what the code already says.
Bug: T285483
Change-Id: I81fa8a5d9d0752f3aeac4015c9a27b50e054d4df
This patch also marks 2 methods as @private that are not and
should not be used outside of this class.
Bug: T285483
Change-Id: I8a8ffc4868a369b5c47068beb0e83f023872543d
This reverts the revert commit d47b95eb4a.
When no `paramOrder` is given, known parameters should appear in the
order returned from the TemplateData API.
Previously, when TemplateData was present but no paramOrder
specified, then the parameters would appear in alphabetical order as
"unknown" parameters. Now they will appear in the order listed in
TemplateData. This is similar to the fully-specified behavior when
paramOrder is present.
This will only affect the Visual Editor template dialog, and has no
effect on serialization.
Bug: T274545
Change-Id: If8315781572af688ea1c1b14b3694b828f076b4a
The results show that parameter order always follows the appearance
in the template invocation, regardless of `paramOrder`, whether the
parameters are aliased, or whether there are unknown params.
Bug: T285382
Change-Id: I76c6fe8f0a2482cf0856bbafd9f21ba9fc4919a4
When no `paramOrder` is given, known parameters should appear in the
order returned from the TemplateData API.
Previously, when TemplateData was present but no paramOrder
specified, then the parameters would appear in alphabetical order as
"unknown" parameters. Now they will appear in the order listed in
TemplateData. This is similar to the fully-specified behavior when
paramOrder is present.
This will only affect the Visual Editor template dialog.
Bug: T274545
Change-Id: I32538de07641c288081042a41fe39eedfed7d939
Note that the tests expose a bug, getAllParametersOrdered fails to
list an unused parameter. Fixed in I32538de07641c.
Also, a minor fix to avoid an impossible template spec: paramOrder
must include all parameters.
Bug: T274545
Change-Id: Icfa7a765773d04ef05a76ecc09467305e311f6cb
This patch is mostly about the arrow syntax.
Some places can not be updated because the arrow syntax also
changes the meaning of `this.…`, but some code relies on that.
Change-Id: Ida3ab0e0950a428fbd1a85f281013778ee879df4
New changes:
2cbc5f9b2 Update OOUI to v0.41.2
e4e467a6b Use wrapper paragraphs in empty branch nodes
Bug: T65356
Change-Id: I222824c53e43f587c999b6478bec52ef686fed7e
In QUnit 2, QUnit.setup()/QUnit.teardown() were renamed, to be called
QUnit.beforeEach()/QUnit.afterEach(). Though we are insulated by this
through MediaWiki's wrapper for backwards compatibility, changing the
names of the functions we pass to the new names allows us to drop the
old ones.
Bug: T170872
Change-Id: I5bfca33c1d4d920d54c2c54b483be78c61b6d0d7
Previous, reverted attempt: da9b6fffbd.
This attempt also includes 6037fefbe0,
and fixes minor conflicts with other changes.
* In normal images, parse relative 'href' attributes instead of
expanding them to absolute. This resolves Parsoid generating
|link= options for copy-pasted images (T193253).
Keep them in the underscore-form to avoid causing dirty diffs like
T237040 again. Unlike in the previous attempt, we don't need to be
super-careful about the 'resource' attribute, thanks to the Parsoid
changes in T108504.
* In gallery images stuff, prefix the 'resource' attribute with './',
same as normal images do. This causes no functional changes, but it
makes updating tests easier, and the consistency is probably good.
* Update test examples to also prefix 'resource' and relative 'href'
attributes with './', like the real Parsoid does.
Bug: T193253
Change-Id: I91131728a87c9406bf069d46d3c94c9a8905a003
New changes:
c17816c5f Diff sidebar: Make font size slightly smaller
f8439f4cc Deep-freeze linear data
a8919f78e Deep-freeze linear data added by transactions
Local changes:
Fixes for deep-frozen linear model
Bug: T119236
Change-Id: Iae4362c8dab0f2bd335e24498f3e0522b8b1d4fc
Similar to Ic79aba4d4364227c3ecf7fb5411e90532b531f44
This only works if the gallery goes unedited. Probably something needs
to be done in ve.ce.MWGalleryImageNode if we care to be complete.
However, as noted in T214648, the DOM diff'er doesn't traverse into
gallery content and notice these element names. So, it's purely
academic to be doing this anyways.
Bug: T266143
Change-Id: I37799076852fa6f062c9d85bcebb15998fb44a80
Parsoid sometimes emits malformed links (with no 'rel') when a
misnested <figure-inline> tag is moved around. Converting them to
internal links, and adding the 'rel' attribute, makes the element no
longer match in selser, and causes dirty diffs. Alienate them instead.
Bug: T64473
Bug: T150196
Bug: T267282
Change-Id: Ic7b48eb2e61585445a1fb98dc2b516d3b6da3cc4
Make ve.dm.MWTemplateModel#serialize ignore empty parameters if they were not
present in the transclusion before the edit. This avoids dirty diffs where an
user edits a template transclusion via VisualEditor, and the editor adds all
available template parameters to the edit wikitext, even if they were not
changed during the edit.
This logic was ported from the old Wikia-WMF VisualEditor project.[1]
Additionally, add tests for ve.dm.MWTemplateModel serialization.
---
[1] https://github.com/Wikia/app/pull/6450/commits/858eaa9
Bug: T101075
Change-Id: I35f8812724658904d30034db4e4684193a661c1e
The mw:Placeholder attribute semantically means, "don't touch this,"
but french spacing should be freely editable. It's just a funny way
to write a plain wikitext space.
Bug: T254502
Depends-On: Ia164dd1318d45924aa965919e7939c6f817f5d0d
Change-Id: I56e0f0c6526649ea041e023698a48936176dec4b
Wrote code such that only redlinks and normal page urls
using the script path are treated as internal links.
Bug: T248076
Change-Id: Ie476bf7f4b389a659899eab4351c912fc6b24bee
By removing this line, we fall back to the default behavior,
which is to copy the attribute from the original DOM element.
The gallery is supposed to have a class indicating the type (packed,
traditional, etc.). However, Parsoid doesn't care about that and
instead reads the type from 'data-mw'. Instead, changing the attribute
is causing dirty diffs.
Bug: T214649
Change-Id: I96b5a21777046b1caf07a3b1def9fad81bb15939
The previous attempt to fix this didn't preserve any attributes
but removing data-parsoid can result in a loss of wikitext formatting.
This reverts commit bdfd4b6d8f.
Bug: T207325
Change-Id: I2a38e651d17262889eddb149c72c9e08b4e56ed0
When a template does not have user-provided TemplateData documentation,
the TemplateData API falls back to extracting possible parameters from the raw wikitext
to generate an API response with a list of potential parameters. However, it also
sets the "notemplatedata" field in the response, causing the VisualEditor to think
the response contains no useful information and ignore it. This appears to have been
an unintended side-effect of I97a1bfc9f9ead082a673a91b9d2053630a90309c.
This patch ensures that the VisualEditor will correctly consider such responses from
TemplateData by modifying ve.dm.MWTransclusionModel to check if the response contains
a parameter map. Some unit tests were added for the class to verify this behavior.
Bug: T243868
Change-Id: I72005880d9301a53224473900efe2917379e8708
It's supposed to be non-editable but deletable text, like mw:Entity.
We decided to handle them this way in 2015 but never implemented it
(T94509). Currently, accidentally editing text inside of
mw:DisplaySpace node causes the changes to be lost when saving.
Bug: T241906
Change-Id: I78a0cc7a75061a7eefb8b677898b5756326615d6
New changes:
479b50059 Localisation updates from https://translatewiki.net.
c595d8ab0 Metion task related to Firefox hack
0a160fac2 Don't trust selections from the server
d796f3db5 rebaser: Update dependencies
b097dfaad ve.dm.Transaction: Don't translate offsets inside annotate-only replacements
eadee0343 FragmentWindow: Replace previousSelection with initialFragment
561e88158 Use ve.dm.example.imgSrc everywhere
d1dceab31 DesktopContext: Remove onModelSelect event
85947ac55 Pause synchronizer while staging
9a4dd169d Catch various out-of-bounds exceptions
341114afc Remove CE HTML from DM html test fixture
5d3a673e0 ve.ce.Document test: Add src to test image
182ac338e Evalutate fragment selection after staging
e032fa161 rebaser: Drop document opacity while paused
Change-Id: Id551ee2e6510610b8f2e12cf77ce3c8429700872
* In normal images, parse relative 'href' attributes instead of
expanding them to absolute, and parse 'resource' to keep it
identical to 'href' if they refer to the same page (including
same percent-encoding and space/underscore). This resolves Parsoid
generating |link= options for copy-pasted images (T193253).
* In gallery images stuff, prefix the 'resource' attribute with './',
same as normal images do. This causes no functional changes, but it
makes updating tests easier, and the consistency is probably good.
* Update test examples to also prefix 'resource' and relative 'href'
attributes with './', like the real Parsoid does.
Bug: T193253
Change-Id: If2d7f080d9d693568054f8311c1e1b15ca27ea5c
* Handle mw:MediaLinks pointing to to non-existent files, which come
with typeof="mw:Error" (similar to image nodes).
* Fix regression from c66f8e0547, which
caused all mw:MediaLinks to be treated as plain external links again.
* Add test cases.
Bug: T232754
Change-Id: I9ae5bcfc4e24e8c0d22ef77d6a4d03f817fc9768
Parsoid does not use relative links anywhere anymore (T72743). There
is no reason for us to support this. And previous code allowed
"hrefPrefix" to be empty '' sometimes, which is scary, as it could
lead to XSS vulnerabilities if titles starting with 'JavaScript:' are
not handled correctly elsewhere.
Bug: T206357
Depends-On: I8728f63084902c76d1c61193be4367939b069f1a
Change-Id: I99be18877aae2b505cf261bd7cdef6cf0d7a8670
We could generate incorrect links to pages whose title contains a
colon ':' and therefore looks like a fully-qualified URL.
Bug: T206231
Bug: T206357
Change-Id: Ie34694d903a6d97589cc46417f70659559494619
