Page MenuHomePhabricator

Move footnote numbering information out of singleton document cache
Closed, DuplicatePublic

Description

We want to get rid of some technical debt in Cite, caused by a consolidation of footnote numbering into ve.dm.MWDocumentReferences. The problem is that data about the InternalList and refs is being cached outside of the class, so we need to tap into update events and maintain a singleton object which can be retrieved from the document. Then, each ve.dm.MWReferenceNode is reponsible for finding its own numbering in the cached structure.

Instead, we want to push numbering responsibility down into an updateGroups event observer in InternalList, which recalculates numbers and pushes the result into the corresponding InternalItem, where it can be easily accessed by the ve.ce.MWReferenceNode. Top-level indexes will be stored as a 1-element array eg. [ 1 ] and subreference indexes will be a pair of top-level, subref indexes like [ 1, 3 ].

The ve.ce.MWReferenceNode is responsible for turning the index into a content-language (with optional group) rendered number. We still aren't supporting custom group numbering here, for the moment.

Details

Related Changes in Gerrit:
SubjectRepoBranchLines +/-
VisualEditor/VisualEditormaster+26 -29
VisualEditor/VisualEditormaster+2 -1
mediawiki/extensions/Citemaster+7 -9
mediawiki/extensions/Citemaster+23 -37
mediawiki/extensions/VisualEditorwmf/1.45.0-wmf.17+1 -1
mediawiki/extensions/VisualEditormaster+1 -1
VisualEditor/VisualEditormaster+65 -3
VisualEditor/VisualEditormaster+8 -7
mediawiki/extensions/VisualEditormaster+2 -1
VisualEditor/VisualEditormaster+41 -4
mediawiki/extensions/Citemaster+22 -2
mediawiki/extensions/Citemaster+48 -44
mediawiki/extensions/Citemaster+71 -8
mediawiki/extensions/VisualEditormaster+1 -0
VisualEditor/VisualEditormaster+129 -1
VisualEditor/VisualEditormaster+17 -4
VisualEditor/VisualEditormaster+8 -9
mediawiki/extensions/Citemaster+78 -435
mediawiki/extensions/Citemaster+40 -253
VisualEditor/VisualEditormaster+0 -1
VisualEditor/VisualEditormaster+1 -29
mediawiki/extensions/Citemaster+22 -5
VisualEditor/VisualEditormaster+275 -87
mediawiki/extensions/VisualEditormaster+5 -4
mediawiki/extensions/VisualEditormaster+3 -1
VisualEditor/VisualEditormaster+249 -83
VisualEditor/VisualEditormaster+8 -0
mediawiki/extensions/VisualEditormaster+2 -1
mediawiki/extensions/Citemaster+21 -21
VisualEditor/VisualEditormaster+6 -13
VisualEditor/VisualEditormaster+76 -14
VisualEditor/VisualEditormaster+63 -1
mediawiki/extensions/Citemaster+51 -51
Show related patches Customize query in gerrit

Related Objects

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

Change #1161891 had a related patch set uploaded (by Awight; author: Awight):

[VisualEditor/VisualEditor@master] [WIP] add tests for footnote numbering

https://gerrit.wikimedia.org/r/1161891

Change #1161891 abandoned by Awight:

[VisualEditor/VisualEditor@master] [WIP] add tests for footnote numbering

Reason:

squashed

https://gerrit.wikimedia.org/r/1161891

Change #1163406 had a related patch set uploaded (by Thiemo Kreuz (WMDE); author: Thiemo Kreuz (WMDE)):

[VisualEditor/VisualEditor@master] Remove internal, unused ve.dm.InternalList.getItemHtmlQueue

https://gerrit.wikimedia.org/r/1163406

Change #1161454 merged by jenkins-bot:

[VisualEditor/VisualEditor@master] Add named, documented ve.dm.InternalListNodeGroup class

https://gerrit.wikimedia.org/r/1161454

Change #1160690 had a related patch set uploaded (by Thiemo Kreuz (WMDE); author: Awight):

[VisualEditor/VisualEditor@master] Minor refactoring in two ve.dm.InternalList methods

https://gerrit.wikimedia.org/r/1160690

Change #1163406 merged by jenkins-bot:

[VisualEditor/VisualEditor@master] Remove internal, unused ve.dm.InternalList.getItemHtmlQueue

https://gerrit.wikimedia.org/r/1163406

Change #1163779 had a related patch set uploaded (by Awight; author: Awight):

[mediawiki/extensions/Cite@master] [POC] Consume internal item node numbering

https://gerrit.wikimedia.org/r/1163779

Change #1163394 had a related patch set uploaded (by Thiemo Kreuz (WMDE); author: Thiemo Kreuz (WMDE)):

[VisualEditor/VisualEditor@master] [WIP] Add QUnit test for ve.dm.InternalListNodeGroup

https://gerrit.wikimedia.org/r/1163394

Change #1165533 had a related patch set uploaded (by Esanders; author: Esanders):

[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (10fa68ec4)

https://gerrit.wikimedia.org/r/1165533

Change #1166072 had a related patch set uploaded (by Thiemo Kreuz (WMDE); author: Thiemo Kreuz (WMDE)):

[mediawiki/extensions/Cite@master] Make use of beforeEach in MWGroupReferences QUnit test

https://gerrit.wikimedia.org/r/1166072

Change #1166376 had a related patch set uploaded (by Thiemo Kreuz (WMDE); author: Thiemo Kreuz (WMDE)):

[mediawiki/extensions/Cite@master] [WIP] Start using new InternalListNodeGroup class

https://gerrit.wikimedia.org/r/1166376

Change #1165533 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (4d51df7e7)

https://gerrit.wikimedia.org/r/1165533

Change #1166794 had a related patch set uploaded (by Awight; author: Awight):

[mediawiki/extensions/Cite@master] [WIP] Store numbering in MWReferenceNode

https://gerrit.wikimedia.org/r/1166794

Refocusing this task on making Cite's MWDocumentReferences responsible, but putting the data into the DOM nodes.

Change #1169617 had a related patch set uploaded (by Awight; author: Awight):

[VisualEditor/VisualEditor@master] [POC] Emit an event after building the node tree

https://gerrit.wikimedia.org/r/1169617

Tech note: my current challenge is to get the new code to run at the right time:

  • https://gerrit.wikimedia.org/r/1169617 provides an event at the first moment when all footnote numbers could be calculated. But this event arrives before MWDocumentReferences is initialized.
  • The ve.ce.MWReferenceNode is constructed around the same time as its corresponding ve.dm.MWReferenceNode, but we don't know numbers yet.
  • It might make sense to have ref nodes calculate their numbers incrementally as the document is constructed, but this would require some shared state or lookups into other refs eg. for a top-level ref to get a number corresponding to an earlier subref.
  • Don't want to repeatedly assign numbers, this should only happen once when the document is created and once after each editing transaction which touches refs.

Needs discussion, I'm not seeing a simple answer about how to update CE nodes after the document DM nodes are all present.

Change #1170330 had a related patch set uploaded (by Awight; author: Awight):

[mediawiki/extensions/Cite@master] Also store footnote index in MWReferenceNode

https://gerrit.wikimedia.org/r/1170330

Change #1169617 abandoned by Awight:

[VisualEditor/VisualEditor@master] [POC] Emit an event after building the node tree

Reason:

Not needed (yet)

https://gerrit.wikimedia.org/r/1169617

I found a way around the timing issue, using events to notify the CE of renumbering. The proof-of-concept is ready for an initial review to see if I'm going in the right direction.

Change #1163394 merged by jenkins-bot:

[VisualEditor/VisualEditor@master] Add all necessary public methods to ve.dm.InternalListNodeGroup

https://gerrit.wikimedia.org/r/1163394

Change #1174391 had a related patch set uploaded (by Thiemo Kreuz (WMDE); author: Thiemo Kreuz (WMDE)):

[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (276febcab)

https://gerrit.wikimedia.org/r/1174391

Change #1174480 had a related patch set uploaded (by Esanders; author: Esanders):

[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (276febcab)

https://gerrit.wikimedia.org/r/1174480

Change #1174480 abandoned by Esanders:

[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (276febcab)

Reason:

I9f2a08d1c98420197aea2c38fa0e4a0c6d099c66

https://gerrit.wikimedia.org/r/1174480

Change #1174391 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (307c0c15a)

https://gerrit.wikimedia.org/r/1174391

Change #1174746 had a related patch set uploaded (by Thiemo Kreuz (WMDE); author: Thiemo Kreuz (WMDE)):

[VisualEditor/VisualEditor@master] Add all necessary public methods to ve.dm.InternalListNodeGroup

https://gerrit.wikimedia.org/r/1174746

Change #1170330 merged by jenkins-bot:

[mediawiki/extensions/Cite@master] Also store footnote index in MWReferenceNode

https://gerrit.wikimedia.org/r/1170330

Change #1174746 merged by jenkins-bot:

[VisualEditor/VisualEditor@master] Add all necessary public methods to ve.dm.InternalListNodeGroup

https://gerrit.wikimedia.org/r/1174746

Change #1176433 had a related patch set uploaded (by Thiemo Kreuz (WMDE); author: Thiemo Kreuz (WMDE)):

[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (3958b09d4)

https://gerrit.wikimedia.org/r/1176433

Change #1176449 had a related patch set uploaded (by Thiemo Kreuz (WMDE); author: Thiemo Kreuz (WMDE)):

[VisualEditor/VisualEditor@master] Two more fixes for the (existing) InternalList behavior

https://gerrit.wikimedia.org/r/1176449

Change #1176488 had a related patch set uploaded (by Awight; author: Awight):

[mediawiki/extensions/Cite@master] [WIP] Ref node renumbering is driven by the DM

https://gerrit.wikimedia.org/r/1176488

awight renamed this task from Make Visual Editor's InternalList responsible for footnote numbering to Move footnote numbering information out of singleton document cache.Aug 7 2025, 2:38 PM
awight claimed this task.

Change #1176496 had a related patch set uploaded (by Awight; author: Awight):

[VisualEditor/VisualEditor@master] [WIP] Experiment with dropping frozen numbering

https://gerrit.wikimedia.org/r/1176496

Change #1176645 had a related patch set uploaded (by Thiemo Kreuz (WMDE); author: Thiemo Kreuz (WMDE)):

[VisualEditor/VisualEditor@master] Streamline InternalList related logic in VisualDiff

https://gerrit.wikimedia.org/r/1176645

Change #1176654 had a related patch set uploaded (by Awight; author: Awight):

[VisualEditor/VisualEditor@master] [WIP] Shouldn't need to remap keys in new document

https://gerrit.wikimedia.org/r/1176654

Change #1176655 had a related patch set uploaded (by Awight; author: Awight):

[VisualEditor/VisualEditor@master] [WIP] Consolidate InternalList remapping

https://gerrit.wikimedia.org/r/1176655

Change #1176700 had a related patch set uploaded (by Awight; author: Awight):

[VisualEditor/VisualEditor@master] [WIP] Remap internal list group item indexes on transact

https://gerrit.wikimedia.org/r/1176700

Change #1177354 had a related patch set uploaded (by Awight; author: Awight):

[mediawiki/extensions/VisualEditor@master] Remap internalList indexes after creating a new doc

https://gerrit.wikimedia.org/r/1177354

Change #1177355 had a related patch set uploaded (by Awight; author: Awight):

[VisualEditor/VisualEditor@master] Helper function to update a set of groups

https://gerrit.wikimedia.org/r/1177355

Change #1177390 had a related patch set uploaded (by Awight; author: Awight):

[mediawiki/extensions/Cite@master] [WIP] Deprecate MWGroupReferences

https://gerrit.wikimedia.org/r/1177390

Change #1178008 had a related patch set uploaded (by Awight; author: Awight):

[mediawiki/extensions/Cite@master] Renumbering is run from InternalList

https://gerrit.wikimedia.org/r/1178008

Change #1160041 abandoned by Awight:

[VisualEditor/VisualEditor@master] Calculate subref-aware footnote numbers in InternalList

Reason:

ve-core will not known about subrefs.

https://gerrit.wikimedia.org/r/1160041

Change #1178512 had a related patch set uploaded (by Awight; author: Awight):

[mediawiki/extensions/Cite@master] Footnote renumbering is event-driven

https://gerrit.wikimedia.org/r/1178512

Change #1176449 merged by jenkins-bot:

[VisualEditor/VisualEditor@master] Two more fixes for the (existing) InternalList behavior

https://gerrit.wikimedia.org/r/1176449

Change #1176433 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (2824d0c43)

https://gerrit.wikimedia.org/r/1176433

awight added a subscriber: dchan.

A pivotal question has been which codebase will have knowledge about subreferencing. Footnote numbering and other details about Cite data internals have already leaked into ve-core so we need to decide whether these details should be extracted from core or whether we add more knowledge to ve-core. Straddling the two codebases is causing friction.

The proof-of-concept has succeeded in keeping knowledge about subrefs in Cite, but fails to invert the other dependencies from ve-core about how numbering is done. Rather than continue in this direction, we're discussing whether to move the numbering fully into ve-core, along with a base class ve.dm.ReferenceNode (and maybe ve.dm.ReferencesList) which knows nothing about templates, Parsoid, or the UI.

Another complementary approach is to make the VE data model more closely resemble the Parsoid DOM, by moving footnote body content out of InternalList and into "synthetic list-defined" refs in the references list. This empties InternalList and eliminates its special-casing, and allows the references list container to closely match conceptual reference groups and the rendered footnote lists. During migration away from InternalList, the class could proxy to the new containers.

@dchan and @awight are checking with the Editing and Tech Wishes teams before taking any steps towards this larger refactoring.

Change #1160690 merged by jenkins-bot:

[VisualEditor/VisualEditor@master] Minor refactoring in ve.dm.InternalList

https://gerrit.wikimedia.org/r/1160690

Change #1184623 had a related patch set uploaded (by Jforrester; author: Jforrester):

[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (4f51b5f65)

https://gerrit.wikimedia.org/r/1184623

Change #1184623 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (4f51b5f65)

https://gerrit.wikimedia.org/r/1184623

Change #1185982 had a related patch set uploaded (by DLynch; author: DLynch):

[mediawiki/extensions/VisualEditor@wmf/1.45.0-wmf.17] Update VE core submodule to master (a5bd08c8b)

https://gerrit.wikimedia.org/r/1185982

Change #1185982 abandoned by DLynch:

[mediawiki/extensions/VisualEditor@wmf/1.45.0-wmf.17] Update VE core submodule to master (a5bd08c8b)

Reason:

Would need to do the subrepo backport anyway

https://gerrit.wikimedia.org/r/1185982

Change #1185982 restored by DLynch:

[mediawiki/extensions/VisualEditor@wmf/1.45.0-wmf.17] Update VE core submodule to master (a5bd08c8b)

https://gerrit.wikimedia.org/r/1185982

Change #1185982 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@wmf/1.45.0-wmf.17] Update VE core submodule to master (a5bd08c8b)

https://gerrit.wikimedia.org/r/1185982

Mentioned in SAL (#wikimedia-operations) [2025-09-08T20:11:02Z] <kemayo@deploy1003> kemayo, esanders: Backport for [[gerrit:1185991|Enable DT thanks at mediawikiwiki (T400849)]], [[gerrit:1185982|Update VE core submodule to master (a5bd08c8b) (T302413 T391521 T397145 T401890 T402392 T397518 T402717 T403741 T403745)]] synced to the testservers (see https://wikitech.wikimedia.org/wiki/Mwdebug). Changes can now be verified there.

Mentioned in SAL (#wikimedia-operations) [2025-09-08T20:17:50Z] <kemayo@deploy1003> Finished scap sync-world: Backport for [[gerrit:1185991|Enable DT thanks at mediawikiwiki (T400849)]], [[gerrit:1185982|Update VE core submodule to master (a5bd08c8b) (T302413 T391521 T397145 T401890 T402392 T397518 T402717 T403741 T403745)]] (duration: 13m 05s)

Change #1166376 merged by jenkins-bot:

[mediawiki/extensions/Cite@master] Start minimizing/deprecating parts of MWDocument/GroupReferences

https://gerrit.wikimedia.org/r/1166376

Change #1188757 had a related patch set uploaded (by Thiemo Kreuz (WMDE); author: Thiemo Kreuz (WMDE)):

[mediawiki/extensions/Cite@master] Rename "in document order" method to "in reflist order"

https://gerrit.wikimedia.org/r/1188757

Change #1188757 merged by jenkins-bot:

[mediawiki/extensions/Cite@master] Rename "in document order" method to "in reflist order"

https://gerrit.wikimedia.org/r/1188757

Change #1211079 had a related patch set uploaded (by Thiemo Kreuz (WMDE); author: Thiemo Kreuz (WMDE)):

[VisualEditor/VisualEditor@master] Add safe-guard to unstable VisualDiff.freezeInternalListIndices

https://gerrit.wikimedia.org/r/1211079

Change #1211079 merged by jenkins-bot:

[VisualEditor/VisualEditor@master] Add safe-guard to unstable VisualDiff.freezeInternalListIndices

https://gerrit.wikimedia.org/r/1211079