Page MenuHomePhabricator

VisualEditor: Trivial way to get a lot of errors
Closed, ResolvedPublic

Description

  1. Go to https://www.mediawiki.org/wiki/VisualEditor:Testy and click edit
  2. Select all using Ctrl-A (or Cmd-A) and press backspace
  3. Press any content key (e.g. "a")

What should happen:

: You get a document with an "a" in its own paragraph.

What does happen:

: You get a document with an "a" in its own paragraph, and two console errors:

TypeError: this.data[offset] is undefined

...ation)===false){return null;}while(start>0){start--;if(this.offsetContainsAnnota...

load.p...4105Z&* (line 89)

TypeError: this.data[offset] is undefined

...ation)===false){return null;}while(start>0){start--;if(this.offsetContainsAnnota...

load.p...4105Z&* (line 89)

Thereafter, pressing return in the new context gives:

TypeError: node is null

...ve.Node.call(this,type);this.model=model;this.$=$element||$('<div>');this.parent...

Etc.


Version: unspecified
Severity: critical

Details

Reference
bz42219

Event Timeline

*** Bug 42218 has been marked as a duplicate of this bug. ***

After deleting all the content document if completely empty (data.length = 0),
however view still askes what are the annotations at offset 0 - and that's what
fails.
Possible fix it to make method getAnnotationsFromOffset return empty set if
given offset does not exists.

There were actually two cases, with two different root causes.

If the last node in the document was not a paragraph, comment #2 applies: the document is empty and the pre-annotations code was asking for annotations at offset 0. This was fixed a while ago.

If the last node in the document was a paragraph (this is common), the document wouldn't actually be emptied. Instead, all nodes would be removed except the last paragraph, which gets stripped. So after removing "everything", we are left with a document that only contains an empty paragraph. However, in the model tree, this empty paragraph is truly empty and does not contain a zero-length text node (most other empty paragraphs do have one).

Once you type a character, CE pawns it first, processing a transaction that inserts a pawn in the paragraph. The transaction processor gets confused by the lack of any content nodes in the paragraph, and issues a rebuild for the contents of the paragraph, which fails spectacularly. The model tree ends up in an inconsistent state with a text node being a direct child of the document node and the paragraph node being gone. This then caused offset computations to be off by one, which caused other parts of CE to do strange things, culminating in an exception in openAnnotations(). When the exception occurs, the pawn hasn't been unpawned yet, so there's pawn leakage.

Trevor has a commit queued up that will fix this.

Confirmed fixed, will be pushed live on Monday.