From 6ea3a8c6966d6f18190c54e35010c497aea090ab Mon Sep 17 00:00:00 2001
From: Adam Wight <adam.wight@wikimedia.de>
Date: Tue, 10 Sep 2024 11:50:27 +0200
Subject: [PATCH] Try to always limit our document references to the current
 fragment

Reverses the previous logic which traversed up from a fragment to get
the full document's refs.  Much other code in VE isn't ready for this
behavior, for example we can see list-defined refs but not inline refs
defined outside of the fragment.

This patch will ensure that we're only looking at refs accessible from
the current fragment, and prevents caching on fragments because the
cache uses `persistentStorage`, which is shared between fragments and
their parent document.

Bug: T374068
Change-Id: Ia38098f8b3e5a9d24c2206e11edab37d60209225
---
 modules/ve-cite/ve.dm.MWDocumentReferences.js  | 18 ++++++++++++------
 ...ve.ui.MWUseExistingReferenceCommand.test.js |  9 +++++----
 2 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/modules/ve-cite/ve.dm.MWDocumentReferences.js b/modules/ve-cite/ve.dm.MWDocumentReferences.js
index b91f6fb2e..72b9477cc 100644
--- a/modules/ve-cite/ve.dm.MWDocumentReferences.js
+++ b/modules/ve-cite/ve.dm.MWDocumentReferences.js
@@ -39,16 +39,22 @@ OO.mixinClass( ve.dm.MWDocumentReferences, OO.EventEmitter );
 /**
  * Singleton MWDocumentReferences for a document.
  *
- * @param {ve.dm.Document} fragment Source document associated with the
- * singleton, may be a fragment in which case we step up to the original
- * document.
+ * @param {ve.dm.Document} doc Source document associated with the
+ * singleton. May be a fragment in which case we only look at refs included in
+ * the fragment.
+ *
  * @return {ve.dm.MWDocumentReferences} Singleton docRefs
  */
-ve.dm.MWDocumentReferences.static.refsForDoc = function ( fragment ) {
-	const doc = fragment.getOriginalDocument() || fragment;
-	let docRefs = doc.getStorage( 'document-references-store' );
+ve.dm.MWDocumentReferences.static.refsForDoc = function ( doc ) {
+	let docRefs;
+	if ( !doc.getOriginalDocument() ) {
+		// Only use cache if we're working with the full document.
+		docRefs = doc.getStorage( 'document-references-store' );
+	}
 	if ( docRefs === undefined ) {
 		docRefs = new ve.dm.MWDocumentReferences( doc );
+	}
+	if ( !doc.getOriginalDocument() ) {
 		doc.setStorage( 'document-references-store', docRefs );
 	}
 	return docRefs;
diff --git a/tests/qunit/ve-cite/ve.ui.MWUseExistingReferenceCommand.test.js b/tests/qunit/ve-cite/ve.ui.MWUseExistingReferenceCommand.test.js
index eb0f4b6e6..3c2c4f10a 100644
--- a/tests/qunit/ve-cite/ve.ui.MWUseExistingReferenceCommand.test.js
+++ b/tests/qunit/ve-cite/ve.ui.MWUseExistingReferenceCommand.test.js
@@ -2,7 +2,7 @@
 
 QUnit.module( 've.ui.MWUseExistingReferenceCommand (Cite)', ve.test.utils.newMwEnvironment() );
 
-function getFragementMock( hasRefs ) {
+function getFragmentMock( hasRefs ) {
 	const docRefsMock = {
 		hasRefs: () => hasRefs
 	};
@@ -10,7 +10,8 @@ function getFragementMock( hasRefs ) {
 	return {
 		getDocument: () => ( {
 			getOriginalDocument: () => undefined,
-			getStorage: () => docRefsMock
+			getStorage: () => docRefsMock,
+			setStorage: () => undefined
 		} ),
 		getSelection: () => ( {
 			getName: () => 'linear'
@@ -28,6 +29,6 @@ QUnit.test( 'Constructor', ( assert ) => {
 QUnit.test( 'isExecutable', ( assert ) => {
 	const command = new ve.ui.MWUseExistingReferenceCommand();
 
-	assert.false( command.isExecutable( getFragementMock( false ) ) );
-	assert.true( command.isExecutable( getFragementMock( true ) ) );
+	assert.false( command.isExecutable( getFragmentMock( false ) ) );
+	assert.true( command.isExecutable( getFragmentMock( true ) ) );
 } );