[Bug 60117] VisualEditor: Copying references is completely broken
https://bugzilla.wikimedia.org/show_bug.cgi?id=60117 --- Comment #12 from ryasm...@wikimedia.org --- Checked the cases mentioned above.All of them are working now on Betalabs.No JS error is appearing now after copy -paste of reference. -- You are receiving this mail because: You are on the CC list for the bug. ___ Wikibugs-l mailing list Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 60117] VisualEditor: Copying references is completely broken
https://bugzilla.wikimedia.org/show_bug.cgi?id=60117 Ed Sanders changed: What|Removed |Added Status|PATCH_TO_REVIEW |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are on the CC list for the bug. ___ Wikibugs-l mailing list Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 60117] VisualEditor: Copying references is completely broken
https://bugzilla.wikimedia.org/show_bug.cgi?id=60117 --- Comment #11 from Ed Sanders --- This patch may not have been merged into ve-mw yet. Please check commit history. -- You are receiving this mail because: You are on the CC list for the bug. ___ Wikibugs-l mailing list Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 60117] VisualEditor: Copying references is completely broken
https://bugzilla.wikimedia.org/show_bug.cgi?id=60117 --- Comment #10 from ryasm...@wikimedia.org --- The following JS error appears if I copy a reference note with some text following it. Uncaught Error: Unbalanced input passed to document -- You are receiving this mail because: You are on the CC list for the bug. ___ Wikibugs-l mailing list Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 60117] VisualEditor: Copying references is completely broken
https://bugzilla.wikimedia.org/show_bug.cgi?id=60117 --- Comment #9 from Gerrit Notification Bot --- Change 109481 merged by jenkins-bot: Fix balancing of data in cloneSliceFromRange https://gerrit.wikimedia.org/r/109481 -- You are receiving this mail because: You are on the CC list for the bug. ___ Wikibugs-l mailing list Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 60117] VisualEditor: Copying references is completely broken
https://bugzilla.wikimedia.org/show_bug.cgi?id=60117 --- Comment #8 from Gerrit Notification Bot --- Change 109481 had a related patch set uploaded by Esanders: Fix balancing of data in cloneSliceFromRange https://gerrit.wikimedia.org/r/109481 -- You are receiving this mail because: You are on the CC list for the bug. ___ Wikibugs-l mailing list Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 60117] VisualEditor: Copying references is completely broken
https://bugzilla.wikimedia.org/show_bug.cgi?id=60117 --- Comment #7 from Gerrit Notification Bot --- Change 108720 merged by jenkins-bot: Fix up result of selectNodes https://gerrit.wikimedia.org/r/108720 -- You are receiving this mail because: You are on the CC list for the bug. ___ Wikibugs-l mailing list Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 60117] VisualEditor: Copying references is completely broken
https://bugzilla.wikimedia.org/show_bug.cgi?id=60117 Gerrit Notification Bot changed: What|Removed |Added Status|ASSIGNED|PATCH_TO_REVIEW -- You are receiving this mail because: You are on the CC list for the bug. ___ Wikibugs-l mailing list Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 60117] VisualEditor: Copying references is completely broken
https://bugzilla.wikimedia.org/show_bug.cgi?id=60117 --- Comment #6 from Gerrit Notification Bot --- Change 108720 had a related patch set uploaded by Esanders: Fix up result of selectNodes to remove empty items https://gerrit.wikimedia.org/r/108720 -- You are receiving this mail because: You are on the CC list for the bug. ___ Wikibugs-l mailing list Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 60117] VisualEditor: Copying references is completely broken
https://bugzilla.wikimedia.org/show_bug.cgi?id=60117 James Forrester changed: What|Removed |Added Status|PATCH_TO_REVIEW |ASSIGNED -- You are receiving this mail because: You are on the CC list for the bug. ___ Wikibugs-l mailing list Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 60117] VisualEditor: Copying references is completely broken
https://bugzilla.wikimedia.org/show_bug.cgi?id=60117 --- Comment #5 from Gerrit Notification Bot --- Change 108692 merged by jenkins-bot: Fix exception thrown by findEndOfNode https://gerrit.wikimedia.org/r/108692 -- You are receiving this mail because: You are on the CC list for the bug. ___ Wikibugs-l mailing list Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 60117] VisualEditor: Copying references is completely broken
https://bugzilla.wikimedia.org/show_bug.cgi?id=60117 James Forrester changed: What|Removed |Added Target Milestone|--- |VE-deploy-2014-01-30 -- You are receiving this mail because: You are on the CC list for the bug. ___ Wikibugs-l mailing list Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 60117] VisualEditor: Copying references is completely broken
https://bugzilla.wikimedia.org/show_bug.cgi?id=60117 --- Comment #4 from Gerrit Notification Bot --- Change 108692 had a related patch set uploaded by Esanders: Fix exception thrown by findEndOfNode https://gerrit.wikimedia.org/r/108692 -- You are receiving this mail because: You are on the CC list for the bug. ___ Wikibugs-l mailing list Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 60117] VisualEditor: Copying references is completely broken
https://bugzilla.wikimedia.org/show_bug.cgi?id=60117 Gerrit Notification Bot changed: What|Removed |Added Status|ASSIGNED|PATCH_TO_REVIEW -- You are receiving this mail because: You are on the CC list for the bug. ___ Wikibugs-l mailing list Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 60117] VisualEditor: Copying references is completely broken
https://bugzilla.wikimedia.org/show_bug.cgi?id=60117 --- Comment #3 from Roan Kattouw --- Based on these findings, my expectation of what will and won't work is: Probably broken: * Copying a reference by itself, where that reference is immediately preceded by text in the same paragraph (foo|[1]|bar) * Copying a reference plus some text before it (f|oo[1]|bar) * Copying a reference plus some text after it, where there is no text immediately preceding that reference in the same paragraph (|[1]ba|r) Probably works: * Copying a reference plus some text after it, where that reference is immediately preceded by text in the same paragraph (foo|[1]ba|r) * Copying a reference plus text before and after it (f|oo[1]ba|r) * Copying a reference by itself, where there is no text immediately preceding that reference in the same paragraph (|[1]|bar) -- You are receiving this mail because: You are on the CC list for the bug. ___ Wikibugs-l mailing list Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 60117] VisualEditor: Copying references is completely broken
https://bugzilla.wikimedia.org/show_bug.cgi?id=60117
--- Comment #2 from Roan Kattouw ---
I forgot to include how these things lead to the actual failure:
* cloneSliceFromRange returns a slice with an unclosed paragraph opening at the
very beginning, as described above
* The tree build algorithm in ve.dm.Document doesn't actually care about the
unclosed opening and builds the tree as if the mwReference was the first node.
This means we get a mostly valid tree except that all the offsets are off by
one because the paragraph opening element was ignored
* When we try to get the contents of the internalItem associated with the
reference from this unbalanced slice, the range in the tree is one off from the
range in the data array. And so instead of [ {type:'paragraph'}, 'F', 'o', 'o',
{type:'/paragraph'} ] we get [ {type:'internalItem'}, {type:'paragraph'}, 'F',
'o', 'o' ] when calling .getFullData( itemNodeRange, true ) in
ve.dm.MWReferenceNode.static.toDomElements on line 135 (it's actually written
as .getFullData( new ve.Range( itemNodeRange.start, itemNodeRange.end ), true )
which is just WTF and should be simplified)
* This data with two openings and zero closings then gets fed to
getDomSubtreeFromData(), which calls removeInternalNodes(), which spots an
internalItem as the very first element and tries to find its end with
findEndOfNode()
* findEndOfNode() can't find the internalItem closing because there are no
closings, so it tries to throw an error, but that fails because dataElement is
undefined and so accessing dataElement.type is itself an error
--
You are receiving this mail because:
You are on the CC list for the bug.
___
Wikibugs-l mailing list
Wikibugs-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 60117] VisualEditor: Copying references is completely broken
https://bugzilla.wikimedia.org/show_bug.cgi?id=60117
--- Comment #1 from Roan Kattouw ---
Well, not *completely*, just mostly :)
Go to
https://www.mediawiki.org/w/index.php?title=VisualEditor:TestReferenceContents&veaction=edit
and select reference [3] by clicking on it, then press Ctrl+C. You'll get a JS
error.
There are several things wrong here:
In findEndOfNode() inside of ve.dm.Converter.prototype.getDomSubtreeFromData:
* Line 1172 tries to throw an error using dataElement.type, but dataElement
isn't set anywhere, so the generation of the error itself throws an error
In ve.ce.Surface.prototype.onCopy:
* On line 693, cloneSliceFromRange() returns a slice with unbalanced data: [
{type:'paragraph'}, {type:'mwReference'}, {type:'/mwReference'},
{type:'internalList'}, ..., {type:'/internalList'} ]
In ve.dm.Document.prototype.cloneSliceFromRange:
* On both lines 378 and 431, selectNodes() for a range exactly covering a
reference returns the text node as well. In my test case, the return value is [
{ node: TextNode, range: [69,69] }, { node: ReferenceNode } ]. This is clearly
a bug in selectNodes()
* Even if selection was correctly set to something of length 1, line 388 does
not allow for selection[0].range to be undefined, but that's actually expected
when a node is completely selected. This case should be handled, and
selection[0].nodeRange should be checked against range instead in that case. So
something like else if ( selection.length === 1 && range.equalsSelection(
selection[0].range || selection[0].nodeRange ) ) . Otherwise, even if we fixed
the selectNodes bug, the selection.length===1 code path still wouldn't be
taken.
* Because selection.length>1, the else path is taken, and we populate
balanceOpenings and balanceClosings. These two are populated separately in a
way that's broken: because the first node is a text node (not wrapped) we add
an opening for that node's parent, but because the second node is a reference
node (wrapped), we don't add a closing after it. The result is unbalanced
output. This code seems to wrongly assume that leaf nodes are always unwrapped.
--
You are receiving this mail because:
You are on the CC list for the bug.
___
Wikibugs-l mailing list
Wikibugs-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
[Bug 60117] VisualEditor: Copying references is completely broken
https://bugzilla.wikimedia.org/show_bug.cgi?id=60117 James Forrester changed: What|Removed |Added Priority|Unprioritized |Highest -- You are receiving this mail because: You are on the CC list for the bug. ___ Wikibugs-l mailing list Wikibugs-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
