Page MenuHomePhabricator

Determine how best to enable deep linking into an Edit Check of a specific range
Closed, DeclinedPublic

Description

This is needed for Edit Suggestions: the suggestion will guide the user to click to open VisualEditor and jump straight into the Tone Check API with a particular range being checked.

Open question(s)

  • 1. In what proportion of cases must a technical approach match the supplied/expected plain text with the corresponding paragraph in the VE document structure?
    • #TODO: @ppelberg + @KStoller-WMF to propose a threshold here, informed my Growth and Editing engineering teams.
  • 2. How exactly will we validate the paragraph identified in the VE document structured aligns with the supplied/expected plain text? E.g. accuracy method/threshold(s).

Decision to be made

  • 1. What technical approach will we use to locate/identify the desired content within the VE document structure?

Event Timeline

KStoller-WMF added a subscriber: ppelberg.

I'm adding this task to Growth's Epic for tracking purposes, as this is a potential blocking task.
And although this is tagged with Growth, I'm assuming this is work the Editing team will complete. Please correct me if I'm wrong, @ppelberg .

I'm adding this task to Growth's Epic for tracking purposes, as this is a potential blocking task.
And although this is tagged with Growth, I'm assuming this is work the Editing team will complete. Please correct me if I'm wrong, @ppelberg .

+1 to the assumption you've made above, @KStoller-WMF.

The Editing Team will discuss this during this upcoming Monday's planning meeting.

Per today's planning meeting, I'm assigning this over to David C. to document the approach we think would be required to offer this kind of functionality (as well as the tradeoffs inherent with it)

As mentioned in face-to-face discussions, a use case Growth is considering is where ranges are identified in the article's content directly, obtained via REST (without loading it into VisualEditor). It is likely that mapping such ranges to ranges in a VisualEditor session would require some sort of heuristic element (e.g. matching runs of plaintext) which means the accuracy would not be 100% (e.g. due to inline templates). The optimal heuristic parameters would depend on the use cases we hope to support.

Regarding the heuristics, I looked into that a bit in T401185: Create a heuristic for finding a plain text paragraph in VE. I found a small library that seems to do pretty much exactly what we need. So in principle, I consider that particular obstacle solved.

Specifically, I would use something like

function getParagraphsWithTextFromVE() {
	const paragraphs = [];
	const surfaceModel = ve.init.target.surface.getModel();
	const store = surfaceModel.getDocument().getStore();
	const paragraphNodes = surfaceModel.getDocument().getNodesByType( 'paragraph' );
	for ( const paragraphNode of paragraphNodes ) {
		const hash = paragraphNode.element.originalDomElementsHash;
		if ( store.hashStore[hash] === undefined ) {
			continue;
		}
		const paragraphText = store.hashStore[hash][0].textContent.trim();
		if ( paragraphText ) {
			paragraphs.push( {
				node: paragraphNode,
				text: paragraphText,
			} );
		}
	}
	return paragraphs;
}

to get the paragraphs and their plain text. Then use the above mentioned library to identify the right one. And finally use paragraphNode.getOuterRange() to get the ve.Range.

That should work, but closely couples to a lot of VE internals. (Though not something we haven't done before in Add-a-Link and Add-an-Image, I think.)

Main question for me would be: Is this something you want us to do? I do not know what the Editing Team's thinking/policy is around other teams interacting with VE internals.

@Urbanecm_WMF raised the idea: If we had the position of the paragraph in wikitext, first and last character position, would that make it easier to get the ve.Range or does that not help?

It sort of helps, but it doesn't necessarily map closely in either direction. A single point in wikitext could essentially point to a large range in VE after template expansion, or many points in wikitext could all map to the same VE location after e.g. whitespace collapse.

ppelberg updated the task description. (Show Details)

Meta
Per the Editing Team's offline discussion about this today, I've updated the task description with – what we understand to be – the decision we're needing to make in this ticket and the open questions we'll need to answer in order to make it.

The alternative technique we decided on (subclassing) is sufficient for current needs, so this is no longer needed right now.