To make sure also the parent gets a name even if it is not re-used
we need to iterate the list of reference to see if there's at least
one child that matches the parent.
The rest will be taken care of by getUniqueListKey that makes sure
that the matching temporary names will result in matching literal
names.
TODO:
- Write a ve.dm.Convertor test which shows the auto-name being
added.
Bug: T367031
Change-Id: I6ef42c8ffc8a4ff9224bfb2a910682d2c44f0dd2
I believe this was a mistake in the previous patch Idd57997. Using
only the .text() of the parent reference is not sufficient. Think of
a reference that contains a <math> formula or – even worse – is
nothing but a <math> formula. The preview will be empty in this case.
Click handlers in the preview need to be disabled. It's only a
preview.
Bug: T367030
Change-Id: I7e82e03ecfeb4e9cf132985684cff5e191506307
This makes it much easier to deal with the internal auto names
used on new elements created during one edit session.
We're ignoring the correct generation of the auto name literals
for now.
Bug: T367031
Bug: T367030
Change-Id: Idd579970cb64500dac27053213e9b116f23b6d76
This is a direct follow-up for what I started in I3c9b9bb. Again all
this does is moving existing code around. The separate this.deferDoc
property is never used anywhere:
https://codesearch.wmcloud.org/search/?q=deferDoc&files=%5C.js%24
Instead I store the deferred function directly in the target property
this.doc and resolve it when needed.
Note that the concept of a parentDoc still exists. I tried to make
this as obvious as possible via comments and by arranging the code
accordingly:
1. A lot of code calls setDocument which directly sets this.doc. The
parentDoc is never used in this case.
2. The parentDoc is only needed to auto-generate the (empty) document
for the reference when a new reference is created.
That's also why the existing code always calls the constructor with
the parentDoc, even if this ends being unused in many cases: It's a
simple fallback.
Bug: T363096
Change-Id: I7874f187b2b397ed511b695ca9ff95838fcff302
mw.attrs does have a special meaning in this code. Together with
mw.body it reflects 1:1 what was written in the original wikitext
in the <ref …> and <references …> tags. Let's avoid using the same
(or almost same) variable name for other kinds of attributes.
Bug: T363096
Change-Id: I3e599ab910b9639e121f9b9d532b0f57f08f1e73
Introduce a static method so we don't need to copy paste code.
Note that the static method still largely duplicates what the method
.buildIndex() will later do. Both loops iterate the reference groups
and the references in each group. The main difference is that the
"is empty" check stops extremely early the moment it finds any
non-empty group.
That's also why I'm convinced it's not worth caching the result.
I benchmarked it and it's nanoseconds. But there are more reasons:
The non-static .isIndexEmpty() method is currently only used when
Citoid is active the same time. Which means the cached result was
entirely unused on installations without Citoid.
Bug: T356871
Change-Id: Id5c4295086bc977ef52ad141be9962d2eecb1bcc
No need to create an expensive model here when all we need is a
single attribute.
Note this code is still behind a feature flag and not running
anywhere, except on the beta cluster.
Bug: T247922
Change-Id: Iea48837213b72a8d0f4b3cc9d9688d9a08315509
We can re-use the existing this.deferDoc property to achieve the
same. There is apparently no need for a separate "parentDoc"
property for anything else but what can be seen in this patch.
https://codesearch.wmcloud.org/search/?q=%5CbparentDoc%5Cb
And no need to keep neither the parentDoc reference nor deferDoc
callback indefinitely. We can drop it after it got resolved.
This really just moves existing code around without changing
anything.
Bug: T363096
Change-Id: I3c9b9bbdff29a3f35d5a0710b48ca59bf592c6ab
These aren't used anywhere:
https://codesearch.wmcloud.org/search/?q=%5Cbset%28ExtendsRef%7CList%28Key%7CGroup%7CIndex%29%29%5Cb
These setters are reaching into internal details that currently
aren't ever modified from outside of the class, and shouldn't.
If it turns out we ever need to modify one of these we can easily
re-introduce setter functions, but probably give them more
meaningful names.
Bug: T363096
Change-Id: I2c9efc114bdc110a29b8d7fb48ea8c1814cdeb21
Note this is only a bandaid patch that avoids the failure in this
place, but doesn't solve the fundamental problem. It appears like
the data coming from Parsoid (in internalList.….firstNodes and
internalList.….indexOrder) is somehow out of sync, and one of the
numbers in the .indexOrder array points to an element in .firstNodes
that doesn't exist (any more?). I don't know why.
Bug: T351550
Change-Id: Iaa4c748462d90d7783edb89713b61ffff6d70765
The search index is really only used in a single place, in the
buildSearchResults method at the very bottom of the class. I find it
more obvious to understand what's going on when the places where the
search index is populated and used are as close together as possible.
This again really only moves existing code around without actually
changing anything.
We can also drop the extra "built" property and use a special null
value instead. This is possible because we know the only consumer of
the this.index property and can guarantee it can't get confused by
the null.
Bug: T356871
Change-Id: Iaddb3b16b3aa776f89fca2bf0350cce9b6bb1a23
This turns two methods with side-effects into more pure functions
with more obvious input and output. buildSearchIndex rebuilds the
internal search index from the internalList. buildSearchResults
filters and creates the result widgets the user will see.
This patch really only moves existing code around but doesn't change
anything. Except that this.built is set before onQueryChange is
called, not after. This avoids potential endless loops in case
onQueryChange happens to trigger buildIndex again.
Bug: T356871
Change-Id: Ib80a2dcb85779d64bec53caf90c49879d0ea2258
I find this particularly confusing because it makes it look like this is
an array. As a reminder, while empty arrays are false in PHP they are
not in JavaScript. An extra `if ( array && array.length )` is really
critical. But this is a string. Empty strings are false in JavaScript.
No problem.
This was originally written in 2013 via Ib244ff6 as a pure .length
check. The duplication was added a year later via Id401d97 for an
unknown reason.
Bug: T356871
Change-Id: Ied335f170a9a0a7bbc8c8fd12f95b6902f401bbf
Note these are more meant as regression test to better cover what's
done in other patches. Everybody should feel free to delete a test
when it gets in the way. I marked a few especially fragile places
with respective comments.
Bug: T358652
Change-Id: I7844907fe3ef4f3439717381b4ecdac9e2d0a825
This CSS exists since I2ab47e7 from August 2014. The original idea
was to dim the default "General references" when you edit a <ref> or
<references> list in VisualEditor.
Steps to reproduce:
* Start VisualEditor.
* Edit a <ref> or <references> list.
* Edit the group.
* You will see the dimmed text "General references". This is not the
CSS in this patch, but the default styling for OOUI placeholders.
* Open the dropdown. The list will show a "General references" item.
It's not dimmed. This is where the CSS was meant to be.
The CSS class name in the OOUI mixin was actually changed from
"oo-ui-flaggableElement-…" to "oo-ui-flaggedElement-…" via I1abecd8,
just a few days later.
In addition the selector wouldn't work anyway for other reasons.
The dropdown is not inside the `.ve-ui-mwReferenceGroupInputWidget`
container any more but placed outside by the OOUI window manager.
And the selector's specifity is to low, at least since Ic57b3ff.
I argue it's not worth fixing it. Nobody missed it for 10 years.
Light gray text would be illegible anyway on the light gray/light
blue backgrounds used in the dropdown menu. Let's consider it dead
code and just remove it.
The class name doesn't appear anywhere else (any more):
https://codesearch.wmcloud.org/search/?q=flaggableElement
Change-Id: Ia802303737ba35cd4b14fae924b7227472f905fd
This can be quite confusing:
* A node does have attributes. One of the attributes is called
"refGroup", another one "mw".
* mw contains a JSON structure with just a few elements, most
notably a "body" and an "attrs" element. These reflect what was
originally written in the wikitext.
* mw.attrs reflects the original properties a.k.a. attributes from
the <ref …> or <references …> tag.
Deleting mw.refGroup doesn't do anything because the attribute is
called <ref group="…"> in the wikitext, not <ref refGroup="…">.
You can actually see this bug in action on all wikis: Go to a page
that uses references in non-standard groups, e.g.
https://en.wikipedia.org/wiki/Historic_Cherokee_settlements
Start VisualEditor. Find e.g. the [notes 1] reference. Edit it
and change the group from "notes" to "General references". Click
"Publish…" and "Review" your changes. The visual diff works because
it apparently uses other information. The wikitext diff is empty.
This is also what's saved: nothing. The edit is lost.
Bug: T359943
Change-Id: I798605d2fd60a6b8f317ec85a4e4d08fd245e084
This didn't mean what it looked like: `||` has higher priority, so an
undefined elem would not result in an empty string.
Change-Id: I1e361842f060815b04802a1ab8f077faa1a8bc6b
Some of the annotations were used in a way that confused jsdoc. This
cleans up redundant annotations and uses more canonical tags.
These changes cause all classes to now appear in the generated pages.
Includes linking to external docs.
Bug: T358641
Change-Id: Iaee1dadcc19a70c27839d0d27dfa6a07a70fb46b
These tags are 10 years old. Current documentation generators don't
need them.
Tagging something explicitely as being a @method can be useful in
an interface where the elements are initialized with e.g. `= null;`
instead of having an implementation. But we have implementations
here. Sure these are methods. No need to say that in the
documentation.
Also removing a comment that's obviously a copy-paste mistake from
what was the ve.ui.MWMediaSearchWidget back then. See Ib244ff6 and
before.
Change-Id: I7df6c789d10fd89e7fe97d56c942fd22c56d8458
No need to manually scan for the currently selected item. And no
need to do it twice. The feature is a little hidden (calling the
method with no parameter) but always was supported.
Bug: T356871
Change-Id: I02401284eef5687eb0825d8d9ae0df294b3b4e03
I find it very helpful to use the name "mwAttrs" specifically for the
mw.attrs thingy that contains the original key-value pairs from the
wikitext.
An alternative is to use ve.getProp, which is helpful in other
situations.
Change-Id: I3edf0dfe5b9629fcda0bf8cb3b734215377a5c77
This reverts commit 0566a495f3.
Reason for revert: Merged too soon, while discussion of the whole
approach is still ongoing.
Change-Id: I2d3d6455cd4ea12067e2020f6b41cfbb4672bbb5
It's fine to copy attributes directly from the reference node rather
than go through the specialized model object.
Bug: T336417
Change-Id: Idaca192137dc762ddced2ee8446a7d838f97e317
Begin a QUnit test module for the reference model. Tests demonstrate
that a new ref and a normal ref reuse from the full document both
behave as expected.
Bug: T336417
Change-Id: I1337806d41b50329ba971c8e68e1a62b52cc9a52
In articles with references, a reference list is generated. If you
try to visual edit the reference list, it displays a message "This
reference list is generated by a template, and for now can only be
edited in source mode." However it is actually editable now that
T54750 (Community Wishlist 2023 wish #2) is complete.
Bug: T54750
Change-Id: Id8115ae6045f371e4619c85aeb610fe78927d802
* Be more specific about the type of context which a
context item belongs to.
* Make grammar clearer.
Change-Id: I9bcc129766c3386582def0f346d6f175e54d6ff6
This reverts commit da30b2b626.
Reason for revert: Caused a regression. Feature is not enabled on public wikis so reverting is harmless for now.
Bug: T247922
Bug: T340757
Change-Id: I83434afaf1b76425bddb575dd724f462a247c83d
This is a first demo that it is possible to get the text of the parent.
This is hidden behind a feature flag.
Next: see if the parent ref can be added to the rendering
Bug: T247922
Change-Id: Idd409f25b253a19c20ed8623737ecc49315587dc
To warn users when they are editing a sub-reference we
add a warning containing the text of the parent reference.
This is hidden behind a feature flag.
Bug: T247922
Change-Id: I3749683d8a18e502bf16e5bd5f2fe385581625be
* Use OOUI message widget to make code cleaner
* By default message is toggled off and only displayed when needed
* To prevent visible changes the message widget is slightly adapted:
* Use black alert icon to prevent yellow default icon for warning messages
* Remove bold text (in css)
* Some padding to top and bottom
* Changes can easily be removed if message should be closer to the standard
Bug: T247922
Change-Id: I2296cd497c935ea4638650bdb4b3c833a71a6c6a
I assume the code was using lastIndexOf as some kind of performance
optimization. Certain StackOverflow threads suggest it without going
into detail. It's not correct here. You can actually name a group
"mwReference/", which will result in the (valid) internal name
"mwReference/mwReference/". This works as expected with indexOf but
not with lastIndexOf.
Change-Id: I8e85ae5c11a74016c7720fcdb6ac6478431aaa8e
* reads the new attribute extends from wikitext
* saves it into the reference model
* adds a message to the VE popup of an extension as a first demo
* tests will be added in a separate patch
Bug: T247922
Change-Id: If4d309c4678022642f39e21565950dc45e557d47
This means the reference list is always in sync with the model's
understanding of which references are available to edit.
An exception is left in if there are no references in the model
at all, as will be the case on he.wiki, as all references are
template generated. In this case we continue to use the Parsoid
DOM, so that there is a static rendering of the reference list.
Bug: T336865
Change-Id: Iaf1089c9de532e7749c9cb70a9e697917955dca8
Minimal test case:
<ref name=a>a</ref>
<ref name=a/>
<references/>
This renders as "1. ↑ 1.0 1.1 a" in both the legacy parser as well
as Parsoid. The moment you start editing this in VisualEditor the
space before "a" disappears. This patch fixes this inconsistency.
Change-Id: Idfea1a445fc98a0433640b4f706fafcc4e236c18
This fixes a minor inconsistency: A reference that comes from a
template and is already reused outside of the template is only
partially available to VE, and previewed with a warning message
because of this:
"This reference is defined in a template or other generated block,
and for now can only be previewed in source mode."
This was missing in the reuse dialog.
Note this patch is not meant to make any design decision, but to use
the existing design consistently.
You can test this with and without the Citoid extension. It works in
both cases.
Bug: T336372
Change-Id: I962cf111b1882bcd736f1090ca17d2b176495d2f
This fixes a minor regression introduced in Ib003b8a. The problem is
that undefined is not equal to anything in JavaScript, and not smaller
than 2 either.
Bug: T241885
Bug: T335410
Change-Id: Ia6deb291d923b88a08ceac8fbc0efb682e14f358
This patch also updates a second place that does almost the same.
That other place also excludes placeholders. We intentionally don't
do this in this new place.
Bug: T241885
Change-Id: Ib003b8a7bbe247db6f7da0a4efcfd4e5967fd033
This issue was introduced in I7a8ec9f. I tried to understand what this
does but can't see an effect. The behavior and appearence seems to be
identical with and without this code. In case it turns out this is dead
code (e.g. not needed any more with our current browser requirements)
it should be removed. Otherwise the selector fixed.
Change-Id: I0bfae951aa4543c528dd7e04c00a0e978f1ce49e
This is a prerequisite before any work related to T52568 (being able
to manually name references in VisualEditor) can start.
Why these names should not be hidden:
* We don't know if the name is actually part of the auto-generated
sequence in the current article or copy-pasted from somewhere else.
* Manually given names that start with a colon are currently hidden
even if they are unrelated to the auto-generated sequence.
* The information is highly relevant for users switching between VE
and wikitext. Especially when a reference is used multiple times
the relevant wikitext can be as short as <ref name=":0" />. The
literally only information in this case is the number.
Since these numbers are still more technical than anything we make
them very dim to emphasize the contrast to non-numeric names.
Bug: T52568
Bug: T92432
Change-Id: I65cb6998cb5f8659cd9043f3d4aaeac1c5f69da8
This original use case at T187495 is for he.wiki, where it is likely
that all the references are defined in templates, so the model
will always be empty, even though $originalRefList is populated.
Change-Id: Ia2785a20bf82ab97466276a57936bc9299e1cabe
