This update better detects inserted text, by disregarding HTML tree structure (which isn't very relevant for these purposes).

I think these are good suggestions. I'd also suggest adding an Indic script.

Oh, probably I'm not being clear. I was thinking:

• we want to tag edits that should contain a reference (according to our heuristic)
• regardless whether or not they do contain one
• and regardless whether the reference check UI is deployed

On reflection, I suggest we consider removing requirement 1B ("the new paragraph does not include a reference"), because:

We're currently envisaging two uses for sentence segmentation in Edit Check:

Based on investigations, I believe Change 881705 fixes the likeliest occurrences of this Gboard bug. I don't think it breaks any case that works at the moment, even though it allows the user to experience T330284 (which needs a separate fix).

Technical notes copied from T325129:

Yes, here is a viable fixup for the above case:

1. On an input event of inputType 'deleteContentBackward', save the current cursor position (but only if the selection is collapsed).
2. If a subsequent keyup event occurs during the same tick, and lands inside a contentEditable=false node, then restore the saved cursor position.
3. In any case, clear the saved cursor position at the end of the tick (i.e. using setTimeout).

The fixup has to occur in the keyup event: waiting for the more logical selectionchange event means the soft keyboard has already closed and it's too late.

This appears to happen because English Gboard + Android Chrome lets the cursor land inside a contenteditable=false span. It does not happen with certain other Gboard languages (e.g. Cantonese). Below is a minimal test case.

Change 812093 Patch set 5 handles the <div><br></div> but fails in other cases (e.g. when the native action adds a list item).

Ok, change 812093 triggers the VE Enter handler — which is a big improvement! As it stands, it still leaves a spurious <div><br></div>, into which we can drag the cursor and type text which then doesn't get put into the model (see screenshot). We should check for this spurious div and clean it up — and as a bonus, we can condition triggering the VE Enter handler on finding this div, so that if another IME behaves as we want it to already, then it won't be affected.

The video below shows the bookmarklet used to run undeployed code against a live mediawiki instance:

On a quick test I see a ~5x speedup, testing on 809052 and 909093 above.

It's done because a model change within a ContentBranchNode gets propagated to the view by replacing the entire contents of the CBN, which resets the native selection to the start of the CBN.

How about "topic is renamed"?

Deleted text is supported by resolving T230325 . Deleted DOM nodes need T230323 (which does not fix deleted text).

Glitch behaviour for the 754986 option

@matmarex coded the approach of styling the link node with "white-space: no-wrap", then adding an inner span styled as "white-space: normal". See here: https://gerrit.wikimedia.org/r/c/VisualEditor/VisualEditor/+/754986/

Confirmed that this happens even with the cursor outside the link cartouche. It happens because the paste is calculated entirely within the DM, and the DM cursor positions do not specify on which side of the annotation boundary they lie.

Some brief points from our conversation just now. It's great that you're looking at this issue early on, so thank you!

Hmm, can't reproduce the behaviour on iOS 12. Can you give a specific minimal example (exactly what content, where to click etc)?

I think the second patch means you may have to tap again sometimes to get into the link, but that's better than not being able to drop the cursor outside the link at all.

Without the toolbars, the interface is less unwieldy and easier to navigate, especially on mobile:

Hmmm I'm not seeing this as a regression — I get the same behaviour going right back to 2016. Investigating further.

This is due to a Firefox issue. A minimal test case for it is:

If I've understood the meaning of the diagram above correctly, I don't think the fallback case will ever happen — because LanguageConverter will always return output for whichever zh variant is specified.

Further detail (from discussing with @DLynch): it's not clear what document data a top-level wrapper could usefully expose as a Vue-style reactive property.

Ah, i see ... with TreeModifier, pressing Enter at the end of a paragraph leaves that paragraph unchanged (whereas before TreeModifier, it was getting rebuilt entirely). Then it moves the focus into the new paragraph below. That means we're escaping the paragraph without cleaning up the unicorns, which the current code assumes to be impossible.

As discussed, I'll base on https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/DiscussionTools/+/578387/ and use the https://en.wikipedia.org/api/rest_v1/page/lint api.

The table selection update is incorrect, because we're calling tx.translateRange with excludeInsertion=false, but there should always be excludeInsertion=true for a table.

The technique in https://gerrit.wikimedia.org/r/539074 does work. And I've used that patchset a few times now to debug silent errors. But it's perhaps too hacky to merge even for debug mode only (it shadows each jQuery promise with a native promise). Maybe it could be merged but needing specific activation.

Ooh, thanks!

Apologies @sbassett , I judged the patch wasn't particularly revealing but maybe that wasn't really my call. Given it's already up, should we now proceed as we would with a non-security-related UBN?

This patchset should fix ithe security issue: https://gerrit.wikimedia.org/r/553888

Yes that looks like the same pattern – thanks for the additional info!

Technical details for triage: this is triggered if a transaction contains a removal starting in the interior of a text node, crosses an inline element, and moves immediately into another text node, and then is followed by a subsequent retain and removal. See the patch set above for a minimal example.

I can no longer reproduce this on https://en.wikipedia.org/w/index.php?title=GC_skew&oldid=917245666 – Adding a link annotation to "locations plotted 5′ to 3′ and y-axis" appears to work fine.

Change 541995 fixes the real-life example above.

Steps to reproduce real-life example (from T234142):

Example 3 is not fixed by Change Set 541088. It is a different bug, which I've filed separately as T234881 .

Change 541088 appears to fix every failure case given in the subtasks. (I loaded them all up on the live sites, in-browser patched the code, and cofirmed they no longer fail).

On investigation, all subtasks are failing for precisely the same reason, so we should probably fold them into this task.

Ok, I've added the extra emit method, but with the name emitThrow rather than emitSync, since that makes clearer how this method differs from plain emit: see https://gerrit.wikimedia.org/r/#/c/oojs/core/+/529726/8/src/EventEmitter.js .

I filed a separate task about using window.onerror (T230441)