Page MenuHomePhabricator

Introduce DM events for "a node of this type was inserted/deleted/modified"?
Closed, ResolvedPublic8 Story Points

Description

There are a few contexts in which we've thrown around the idea of attaching an onDocumentTransact listener that goes through the inserted/deleted data in each transaction and checks if a node of a certain type was added/removed:

  • Reference list updates (which currently happen on every transaction IIRC)
  • Adapting VE-MW to @dchan's metadata patch (specifically, to update the model of which categories are on the page and in what order)
  • Something about images that @Jdforrester-WMF mumbled in my general direction

Do we think it makes sense to have a core DM feature that does something like the following?

this.document.on( 'transact', function ( tx ) {
    for each operation {
        for each inserted or deleted data item {
            this.emit( 'itemModified', item.type );
        }
   }
} );

Or perhaps something that maintains its own set of listeners so that we can skip this if we know there aren't any?

Thoughts?

Event Timeline

Catrope created this task.Apr 12 2017, 1:15 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptApr 12 2017, 1:15 AM
dchan added a comment.Apr 12 2017, 2:18 AM

Interesting question. On the tree side of things, I've been considering the shortcomings of detecting ancestry modifications (e.g. see https://gerrit.wikimedia.org/r/#/c/345955/4/src/ce/nodes/ve.ce.ContentEditableNode.js ). It isn't a huge issue right now, because we rebuild branches (ve.dm.Document#rebuildNodes) rather than altering them, so the only ancestry modifications we support are birth and death. But if in the future we change ve.dm.TransactionProcessor to be able to graft branches (see T162762 ), it will become more important.

I wonder a single new feature could help with both these use cases.

Jdforrester-WMF triaged this task as High priority.Apr 18 2017, 7:27 PM
Jdforrester-WMF moved this task from To Triage to TR3: Language support on the VisualEditor board.
Jdforrester-WMF set the point value for this task to 8.

This is a blocker for T56299 landing (because of the VE-MW need); is there a way we could implement this inefficiently at first and then make it better/faster after T162762? I really want that landed soon…

Change 350965 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[VisualEditor/VisualEditor@master] [WIP] Allow document subscribers to be informed to changes to nodes

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

dchan claimed this task.Apr 29 2017, 3:33 AM

Change 377237 had a related patch set uploaded (by Divec; owner: Divec):
[VisualEditor/VisualEditor@master] Notification for DM node attached/detached

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

Change 350965 abandoned by Divec:
[WIP] Allow document subscribers to be informed to changes to nodes

Reason:
Abandoned in favour of https://gerrit.wikimedia.org/r/377237/ (see above for detailed explanation)

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

Change 377237 merged by jenkins-bot:
[VisualEditor/VisualEditor@master] Notification for DM node attached/detached

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

Change 383943 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (df62df432)

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

Change 383943 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (df62df432)

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

Deskana closed this task as Resolved.Oct 13 2017, 9:35 AM
Restricted Application added a project: User-Ryasmeen. · View Herald TranscriptOct 13 2017, 9:35 AM