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
This commit is contained in:
Adam Wight 2024-09-10 11:50:27 +02:00
parent e26e726cb0
commit 6ea3a8c696
2 changed files with 17 additions and 10 deletions

View file

@ -39,16 +39,22 @@ OO.mixinClass( ve.dm.MWDocumentReferences, OO.EventEmitter );
/** /**
* Singleton MWDocumentReferences for a document. * Singleton MWDocumentReferences for a document.
* *
* @param {ve.dm.Document} fragment Source document associated with the * @param {ve.dm.Document} doc Source document associated with the
* singleton, may be a fragment in which case we step up to the original * singleton. May be a fragment in which case we only look at refs included in
* document. * the fragment.
*
* @return {ve.dm.MWDocumentReferences} Singleton docRefs * @return {ve.dm.MWDocumentReferences} Singleton docRefs
*/ */
ve.dm.MWDocumentReferences.static.refsForDoc = function ( fragment ) { ve.dm.MWDocumentReferences.static.refsForDoc = function ( doc ) {
const doc = fragment.getOriginalDocument() || fragment; let docRefs;
let docRefs = doc.getStorage( 'document-references-store' ); if ( !doc.getOriginalDocument() ) {
// Only use cache if we're working with the full document.
docRefs = doc.getStorage( 'document-references-store' );
}
if ( docRefs === undefined ) { if ( docRefs === undefined ) {
docRefs = new ve.dm.MWDocumentReferences( doc ); docRefs = new ve.dm.MWDocumentReferences( doc );
}
if ( !doc.getOriginalDocument() ) {
doc.setStorage( 'document-references-store', docRefs ); doc.setStorage( 'document-references-store', docRefs );
} }
return docRefs; return docRefs;

View file

@ -2,7 +2,7 @@
QUnit.module( 've.ui.MWUseExistingReferenceCommand (Cite)', ve.test.utils.newMwEnvironment() ); QUnit.module( 've.ui.MWUseExistingReferenceCommand (Cite)', ve.test.utils.newMwEnvironment() );
function getFragementMock( hasRefs ) { function getFragmentMock( hasRefs ) {
const docRefsMock = { const docRefsMock = {
hasRefs: () => hasRefs hasRefs: () => hasRefs
}; };
@ -10,7 +10,8 @@ function getFragementMock( hasRefs ) {
return { return {
getDocument: () => ( { getDocument: () => ( {
getOriginalDocument: () => undefined, getOriginalDocument: () => undefined,
getStorage: () => docRefsMock getStorage: () => docRefsMock,
setStorage: () => undefined
} ), } ),
getSelection: () => ( { getSelection: () => ( {
getName: () => 'linear' getName: () => 'linear'
@ -28,6 +29,6 @@ QUnit.test( 'Constructor', ( assert ) => {
QUnit.test( 'isExecutable', ( assert ) => { QUnit.test( 'isExecutable', ( assert ) => {
const command = new ve.ui.MWUseExistingReferenceCommand(); const command = new ve.ui.MWUseExistingReferenceCommand();
assert.false( command.isExecutable( getFragementMock( false ) ) ); assert.false( command.isExecutable( getFragmentMock( false ) ) );
assert.true( command.isExecutable( getFragementMock( true ) ) ); assert.true( command.isExecutable( getFragmentMock( true ) ) );
} ); } );