Commit graph

166 commits

Author SHA1 Message Date
jenkins-bot 4a5f811c82 Merge "Keep template parameter position when resolving aliases" 2021-07-05 11:44:19 +00:00
Andrew Kostka 6af13f0d42 Fix parameter ordering when using aliases
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
2021-07-05 08:14:42 +00:00
Thiemo Kreuz 8d0a623f40 Keep template parameter position when resolving aliases
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
2021-07-03 20:14:06 +02:00
Thiemo Kreuz 280ba23e2c Add missing fail-safe to MWTemplateSpecModel.extend()
I don't want this code to crash when the TemplateData API
returns an unexpected result.

Bug: T285483
Change-Id: I237cbfbb85892a53a08d9e7e34cf4974775d627a
2021-07-02 17:52:43 +02:00
Thiemo Kreuz 07344fce0f Rename misleading "extend" method in template spec class
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
2021-07-02 11:26:04 +00:00
jenkins-bot 2e4ac455f0 Merge "Add more edge-case test cases for template spec class" 2021-07-01 18:57:51 +00:00
Thiemo Kreuz 99523b855c Use and document the term "known parameter" in template code
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
2021-07-01 12:03:38 +00:00
Thiemo Kreuz d8f4da199d Add more edge-case test cases for template spec class
Bug: T285483
Change-Id: I0a9bf7fe51242161dbc57c9301f360c9e8f35b93
2021-07-01 10:13:00 +02:00
jenkins-bot 127ea9ea27 Merge "Avoid the term "canonical order" in template related docs" 2021-06-30 11:35:27 +00:00
jenkins-bot 5943b5a25e Merge "Fix spec.fillFromTemplate() not skipping aliases any more" 2021-06-30 11:11:24 +00:00
jenkins-bot 6aaec0c88b Merge "Rename temporary "x" variable in spec class tests" 2021-06-30 11:09:54 +00:00
Thiemo Kreuz 84329a40c8 Rename temporary "x" variable in spec class tests
I forgot to give this variable a proper name. The "x" was not
meant to stay.

Bug: T285483
Change-Id: I482b9282676082f44d5f98a574e5ada05dfb6318
2021-06-30 12:16:50 +02:00
Thiemo Kreuz 466e428a81 Fix spec.fillFromTemplate() not skipping aliases any more
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
2021-06-30 10:14:43 +00:00
Thiemo Kreuz bc4aeed86e Avoid the term "canonical order" in template related docs
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
2021-06-30 09:38:28 +02:00
Thiemo Kreuz 8dca99d38c Fix broken template serialization test setup
This was not really testing anything before. Introduced in
I76c6fe8.

Bug: T285382
Change-Id: I5f8343c9805b9a1adc2efdd13a103a7f1196e15b
2021-06-29 16:29:11 +02:00
Thiemo Kreuz b7d88c541b Fix all spec methods to not crash on unknown parameters
Change-Id: I16708b048a785f0712084bd2d087e4aab77fb72b
2021-06-25 08:36:23 +02:00
Thiemo Kreuz b8382513fc Use separate data structure to store aliases in spec
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
2021-06-25 08:29:48 +02:00
Thiemo Kreuz 8f6098c03b Simplify spec code dealing with default values
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
2021-06-24 18:13:41 +02:00
Thiemo Kreuz bab08440ea Fix getDescription/Sets possibly returning undefined
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
2021-06-24 18:07:46 +02:00
Thiemo Kreuz 2529d33f51 Better name for spec.isParameterKnown() method
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
2021-06-24 18:05:14 +02:00
Thiemo Kreuz 30dc85f53b Better name for spec.getParameterName() method
This reflects much better what the method is for.

Bug: T285483
Change-Id: I7c90643421e32946fce4de813c7614b806b261f0
2021-06-24 18:02:52 +02:00
Thiemo Kreuz 3d02c1f364 Add new QUnit test for ve.dm.MWTemplateSpecModel
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
2021-06-24 17:59:38 +02:00
Adam Wight 1909fdff92 Revert "Revert "Fall back from explicit parameter order to TemplateData sort""
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
2021-06-23 16:39:13 +02:00
Adam Wight 05bcb89f3f Tests for parameter serialization order
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
2021-06-23 16:38:30 +02:00
Awight d47b95eb4a Revert "Fall back from explicit parameter order to TemplateData sort"
This reverts commit 12f3ce5ed3.

Reason for revert: Considered undesirable, per T138200

Change-Id: I089a47b41a0bb0405841690ca577035cf95a8016
2021-06-22 17:50:16 +00:00
Adam Wight 12f3ce5ed3 Fall back from explicit parameter order to TemplateData sort
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
2021-06-22 13:24:44 +02:00
Adam Wight 33701872c7 Tests for getAllParametersOrdered
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
2021-06-22 13:23:59 +02:00
Adam Wight 9e1c76f0fc Tests for MWTemplateModel parameter ordering
Documents existing logic.

Bug: T274545
Change-Id: Iebcbdb186862060724d29b36112b2d262112c4c8
2021-06-18 13:43:15 +02:00
Thiemo Kreuz 346216c050 Update more test code to use ES6 syntax
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
2021-04-30 10:08:45 +00:00
Thiemo Kreuz 2611f907a9 Update all tests in the ve-mw module to use ES6 syntax
Change-Id: I75f8864c66c9b2a78c683d425d54a94515b74f24
2021-04-30 08:07:54 +00:00
Bartosz Dziewoński d23045bb0b Update VE core submodule to master (145cf95d8)
New changes:
2cbc5f9b2 Update OOUI to v0.41.2
e4e467a6b Use wrapper paragraphs in empty branch nodes

Bug: T65356
Change-Id: I222824c53e43f587c999b6478bec52ef686fed7e
2021-03-11 16:00:01 +00:00
jenkins-bot cb21c4a158 Merge "Parse relative hrefs on image nodes like on regular links (try 2)" 2021-02-24 17:19:44 +00:00
James D. Forrester d315a3af88 tests: Align setup/teardown function naming with QUnit 2.x
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
2021-02-05 11:34:05 -08:00
jenkins-bot 6a8350b92b Merge "ve.dm.MWTemplateModel: Don't add spurious empty parameters" 2021-01-27 16:31:10 +00:00
Bartosz Dziewoński 91291d3e98 Parse relative hrefs on image nodes like on regular links (try 2)
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
2021-01-27 11:53:15 +00:00
Ed Sanders f8c403dffb Update VE core submodule to master (a8919f78e)
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
2021-01-23 19:55:14 +00:00
Arlo Breault 895274c20b Switch back to using <span> for gallery images
Similar to I74fe96a47d8a4d7717891c16e49f5a4d6599018a

Bug: T266143
Change-Id: I632cff28ec428828d82fcd7cc66e5545c45c2161
2020-12-15 15:43:48 -05:00
Arlo Breault e3daaaf280 Preserve the passed in inline media tag name in gallery
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
2020-12-15 14:42:00 -05:00
Ed Sanders 5e170b63ca Switch back to using <span> for inline images
Bug: T266143
Change-Id: I74fe96a47d8a4d7717891c16e49f5a4d6599018a
2020-12-13 18:02:06 +00:00
Arlo Breault 6acddba351 Preserve the passed in inline media tag name
Bug: T266143
Change-Id: Ic79aba4d4364227c3ecf7fb5411e90532b531f44
2020-12-10 18:19:35 -05:00
Bartosz Dziewoński d52097cfdb ve.dm.MWExternalLinkAnnotation: Alienate malformed links
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
2020-12-07 13:56:43 +00:00
Máté Szabó d127dc48b7 ve.dm.MWTemplateModel: Don't add spurious empty parameters
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
2020-11-16 19:54:44 +01:00
C. Scott Ananian ed3579245e French spacing (mw:DisplaySpace) doesn't have mw:Placeholder any more
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
2020-07-15 20:14:30 +00:00
Ed Sanders 877d1e3359 Move restbaseId stripping to ve.utils.parsoid
De-duplicates the two implementations we had.

Bug: T253584
Change-Id: If5d15dd4e7e0d3704ca8d75c0a25c529b06f17fb
2020-06-10 20:31:16 +01:00
Ed Sanders 6076708ff3 build: Update eslint-config-wikimedia to 0.16.0
Change-Id: I638e0f82949597e2a2e4ea18fc2f0258f225358c
2020-06-02 21:30:00 +01:00
Sohom Datta d16d93ccf7 Treat links to diffs and history pages as external links
Wrote code such that only redlinks and normal page urls
 using the script path are treated as internal links.

Bug: T248076
Change-Id: Ie476bf7f4b389a659899eab4351c912fc6b24bee
2020-04-17 22:48:14 +05:30
Bartosz Dziewoński 26062fd3a0 ve.dm.MWGalleryNode: Preserve 'class' attribute unchanged
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
2020-02-14 22:05:50 +01:00
Esanders c9b2e8b4d2 Only preserve data-parsoid/RESTBase ID on transclusion nodes
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
2020-02-12 22:51:43 +00:00
Bartosz Dziewoński a816baedd9 Fix handling of pasted internal red links
Bug: T239550
Change-Id: Iacfba4b46bea8294f12a0c010344fda317f75df6
2020-02-07 17:41:19 +01:00
jenkins-bot 582b99781c Merge "Change gallery structure to match Parsoid" 2020-02-05 00:52:51 +00:00