Last modified: 2012-11-26 17:28:08 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T44219, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 42219 - VisualEditor: Trivial way to get a lot of errors
VisualEditor: Trivial way to get a lot of errors
Status: RESOLVED FIXED
Product: VisualEditor
Classification: Unclassified
ContentEditable (Other open bugs)
unspecified
All All
: High critical
: VE-deploy-2012-11-26
Assigned To: Roan Kattouw
:
: 42218 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-11-17 02:54 UTC by James Forrester
Modified: 2012-11-26 17:28 UTC (History)
7 users (show)

See Also:
Web browser: Firefox
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description James Forrester 2012-11-17 02:54:33 UTC
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.
Comment 1 Inez Korczyński 2012-11-17 07:46:44 UTC
*** Bug 42218 has been marked as a duplicate of this bug. ***
Comment 2 Inez Korczyński 2012-11-17 07:49:50 UTC
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.
Comment 3 Roan Kattouw 2012-11-21 20:35:27 UTC
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.
Comment 4 Roan Kattouw 2012-11-21 22:03:45 UTC
https://gerrit.wikimedia.org/r/#/c/34597/1
Comment 5 James Forrester 2012-11-21 23:50:40 UTC
Confirmed fixed, will be pushed live on Monday.

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links