Commit graph

752 commits

Author SHA1 Message Date
Trevor Parscal d602724e83 Stop using ve.isMixedIn() to check if a node is focusable
* Moved isNodeFocusable to ve.ce.NodeFactory
* Added isFocusable static property to ve.ce.Node
* Set isFocusable to true on ve.ce.FocusableNode

Change-Id: I3cf666280abdfce55bf9b0710827bb25c40bfd51
2013-10-11 16:00:34 +02:00
Roan Kattouw 75270e24d5 Fixes for Trevor's crazy half-baked commit
Fix things that 4aa86d0f8 broke:
* Update surface parameter to windowSet in all ve.ui.SurfaceDialog subclasses
* Do the same for ve.ui.SurfaceInspector subclasses
* Fix @extends documentation for SurfaceDialog
* Fix documentation for ve.ui.SurfaceInspector, copypasta from SurfaceDialog

Bonus:
* Add .getMetaList() getter to dm.Surface

Change-Id: I843e99e45e9b013cb9cb559f050384d39bbbddf2
2013-10-09 22:08:13 +02:00
Ed Sanders a8d84db0e8 Refactor out data processing from ve.dm.Document constructor
Also make collection of metadata and construction of nodes optional.

Change-Id: I02ba6d2199caccaf9fe9dcfba58eefa7b52c52b1
2013-10-07 17:26:21 +01:00
Ed Sanders 44b1fdebe4 Use FlatLinearData for storing converter results
Previously we returned ElementLinearData from the converter, then
stripped out the MetaLinearData. This meant that before processing
the ElementLinearData from the converter actually contained metadata
which is confusing.

The new document constructor stores the converter results in a
FlatLinearData object and simultaneously populates element and meta
data stores.

Also in this commit I have moved various methods from ElementLinearData
to FlatLinearData, from which ElementLinearData inherits.

Change-Id: I64561bde2c31d8f703c13ac7b0a0c5f7ade9f3d4
2013-10-06 20:27:32 +01:00
jenkins-bot b36c401ec7 Merge "Get rid of 'reversed' flag on transactions" 2013-10-04 06:18:06 +00:00
jenkins-bot c21c8556fc Merge "Make ve.Factory require static name property" 2013-10-03 22:03:26 +00:00
Trevor Parscal 61ddfb76e4 Make ve.Factory require static name property
Objective:
* Make ve.Factory behave like ve.NamedClassFactory
* Remove the only remaining use of ve.Factory (actions)
* Remove ve.NamedClassFactory

Change-Id: Ie302ef5ea31081de7ab0db6091058a59946aef4c
2013-10-03 14:52:19 -07:00
Roan Kattouw 69ad031bff When cloning the InternalList, pass through properties that aren't rebuilt
InternalList.clone() assumed that all properties are automatically rebuilt
when a new document is built, but that's not true for .nextUniqueNumber
(or for .itemHtmlQueue for that matter). This meant that, in practice,
.nextUniqueNumber was being reset to 0 after auto/N numbers for existing
references had been assigned, but before assigning numbers to newly
created references. This caused all sorts of naming collision fun.

Bug: 54712
Change-Id: I1d087a5f3c23979d7d488e3ab32eb064ebc23e94
2013-10-03 14:43:20 -07:00
jenkins-bot dd9511f249 Merge "Change ve.dm.DocumentSlice to a mixin to ve.dm.LinearData" 2013-10-03 18:45:55 +00:00
Ed Sanders 5c31d3215b Change ve.dm.DocumentSlice to a mixin to ve.dm.LinearData
Document slice only ever contained linear data, with extra functionality
to preserve the range. It pre-dated LinearData, but now we should
refactor it to reflect its purpose.

Change-Id: Ifc908f7526c83a43a51372c8d2494d7260e7facd
2013-10-03 19:38:59 +01:00
jenkins-bot 1a9f6489d7 Merge "Rename getDocumentSlice to cloneFromRange" 2013-10-03 18:19:13 +00:00
Ed Sanders 1957eb3e28 Rename getDocumentSlice to cloneFromRange
We already getSlice which returns a ve.dm.DocumentSlice, so using
the word slice in this method is very confusing. What we are actually
doing is creating a ve.dm.Document from a range. Also remove argument
overloading as it's not particularly helpful and would make the new
name a lie.

Change-Id: I93da3419510410b170396e6765fbe2a87f9795be
2013-10-03 12:48:01 +01:00
Roan Kattouw 6772f92e70 Get rid of 'reversed' flag on transactions
The way we implemented undoing transactions was horrible. We'd process
the original transaction, but with a reversed=true flag. That meant we
had to keep track of the 'reversed' flag everywhere, and use ternaries
like insert = reversed ? op.remove : op.insert; all over the place to
access transaction operations. Redo then worked by reapplying the
transaction. We would verify that this was OK by tracking whether the
transaction was in an applied state or an undone state.

This commit makes it so every transaction can only be applied once. To
undo, you obtain a mirror image of the transaction with tx.reverse(),
then apply that. To redo, you clone the original transaction with
tx.clone() and apply that. All the code that had to use ternaries to
check whether the transaction was being applied in reverse or not is
gone now, because you can only apply a given transaction forwards,
never in reverse.

Bonus:
* Make ve.dm.Document's .completeHistory a simple array of
  transactions, rather than transaction/boolean pairs
* In the protection of double application test, clone the example
  document properly; it modified ve.dm.example.data, which was "fine"
  because it ran .commit() and .rollback() the same number of times

Change-Id: I3050c5430be4a12510f22e20853560b92acebb67
2013-10-02 19:37:08 -07:00
jenkins-bot a3854ad50a Merge "Typing into an annotation next to a word break keeps annotation" 2013-09-30 23:21:29 +00:00
Inez Korczyński e4daeea01e removedItems[i] is already an item, no need to pass removedItems[i].item
Bug: 54727
Change-Id: Iff8e9c8f1dc7a701d4773bf15817897a871d9e98
2013-09-29 17:09:09 +00:00
Ed Sanders f3899c4041 Typing into an annotation next to a word break keeps annotation
Logic was failing because we were passing the index of the annotation
within the AnnotationSet, instead of the index within the Store, to
containsIndex().

Bug: 54332
Change-Id: Ibfd9abe6e4b44d9db744e0c5019418eee12f84a4
2013-09-27 11:53:57 +01:00
jenkins-bot bea113fee2 Merge "Introduce newFromDocumentReplace() transaction builder" 2013-09-26 20:45:54 +00:00
Roan Kattouw cf17789985 Introduce newFromDocumentReplace() transaction builder
Replaces newFromNodeReplacement(). newFromNodeReplacement was very
simplistic and didn't support metadata or internal list items, so
if you had comments or references inside of the data you were editing
(reference contents or an image caption), they'd get mangled.

With this, you can do:
newDoc = doc.getDocumentSlice( node );
// Edit newDoc
tx = ve.dm.Transaction.newFromDocumentReplace( doc, node, newDoc );
surface.change( newDoc );

and that takes care of metadata, internal list items, and things like
references that reference internal list items.

ve.dm.Document.js:
* In getDocumentSlice(), store a reference to the original document
  and the number of items in its InternalList at the time of slicing
  in the created slice. This is used for reconciliation when the
  modified slice is injected back into the parent document with
  newFromDocumentReplace().

ve.dm.InternalList.js:
* Add a method for merging in another InternalList. This provides a
  mapping from old to new InternalList indexes so the linear model data
  being injected by newFromDocumentReplace() can have its InternalList
  indexes remapped.

ve.dm.Transaction.js:
* Replace newFromNodeReplacement() with newFromDocumentReplace()

ve.ui.MWMediaEditDialog.js, ve.ui.MWReferenceDialog.js:
* Use getDocumentSlice/newFromDocumentReplace for editing captions/refs
* Change insertion code path to insert an empty internalItem/caption, then
  newFromDocumentReplace into that
* Add empty internalList to new mini-documents

ve/test/dm/ve.dm.Transaction.test.js:
* Replace newFromNodeReplacement tests with newFromDocumentReplace tests

ve-mw/test/dm/ve.dm.Transaction.test.js (new):
* Add tests for newFromDocumentReplace with mwReference nodes

ve.dm.mwExample.js:
* Add data for newFromDocumentReplace with mwReference tests

VisualEditor.hooks.php:
* Add new test file

Bug: 52102
Change-Id: I4aa980780114b391924f04df588e81c990c32983
2013-09-25 21:46:38 +00:00
jenkins-bot 05ec83657e Merge "Fix variable names in newFromInsertion" 2013-09-25 03:44:27 +00:00
Ed Sanders 0a49efe9e7 Fix variable names in newFromInsertion
Documentation says third argument is 'data' when in fact it is
'insertion' which is then re-used as the result of fixupInsertion :/

Change-Id: I17c959f858eddddc90a6fa839da0d32da69a784f
2013-09-24 20:06:01 -07:00
jenkins-bot 9740d1683e Merge changes I9e065aa4,Iac7649f4
* changes:
  MW*ImageNode's can't take link annotations
  Node annotation blacklists
2013-09-24 00:09:22 +00:00
Moriel Schottlender b8d0807407 LanguageAnnotation Touchup
Fixing a couple of mishaps in the language annotation, including adding a
getComparableObject in the ve.dm.LanguageAnnotation class.

Change-Id: Ia28900df55969f60576cfd716c70ffc1ba5f4c27
2013-09-23 11:41:34 -07:00
Ed Sanders caa05f3315 Fix exception thrown by converter when data ends in text
Conversions usually end in a close tag so this hasn't been a problem
before, but the mini-conversion we do in onCopy may not be balanced.

Change-Id: Ia2db29f116ca84ee886b4c5cafd4ac45cd926b79
2013-09-20 18:26:14 +00:00
Roan Kattouw eb64743436 Prevent naming collisions when generating unique reference names
Simply generating ':3' as the "unique" name for the 4th reference
doesn't work. Even if getUniqueListKey() had been used, that only
checks for conflicts with names that have already been encountered
(i.e. occur in <ref> tags that precede the current one), not for
conflicts with names that first occur further down in the document.

The solution is to generate names at serialization time, when we
have full knowledge of which names are in use. Internally, we use
'literal/<name>' for names that literally appeared in the source,
and 'auto/<number>' for unnamed references. Then at serialization
time, we translate 'auto/<number>' to 'literal/:<number>' if needed
(i.e. if the reference was reused).

ve.dm.MWReferenceNode.js:
* toDataElement()
** Prefix listKey with literal/ or auto/ as appropriate
* toDomElements()
** Map auto/ listKeys to unique names
** Don't try to unset the name if not present (was unsetting a property
   that didn't exist anyway)

ve.dm.InternalList.js:
* Remove now-unused isUniqueListKey()
* Rewrite getUniqueListKey()
** Make prefix configurable
** Take previously generated unique keys into account
** Map the same old key (auto/N) to the same generated key (literal/:M)
* Add getNextUniqueNumber() as a source for auto/N numbers: previously
  used the length of the itemHtmlQueue, but that only works during
  conversion, not from the UI dialog

ve.ui.MWReferenceDialog.js:
* For new references or conflicting names, generate an auto/N key and
  let toDomElements() deal with actually mapping that to name

ve.dm.InternalList.test.js:
* Rename listKeys to new style
* Split the test case into two groups so we can test multi-group cases
* Add tests for getUniqueListKey()

ve.dm.mwExample.js:
* Rename things to new style
* Modify the test case so it attempts to trigger bug 54341

Bug: 54341
Change-Id: I726fb83e6fb66ffec643d996768a854ec9474b3d
2013-09-19 21:03:15 -07:00
Ed Sanders f001d306ad Node annotation blacklists
Allow nodes to specify a blacklist of annotations which can't be applied
to it.

Bug: 53151
Change-Id: Iac7649f4b38d8bfa94c64f734634afd45a5afc53
2013-09-19 19:09:25 +01:00
C. Scott Ananian 7c1ba7750c Minor doc fixes to ve.dm.Transaction.
Correct the return value descriptions for the `newFromMetadataInsertion` and
`newFromMetadataRemoval` methods (which appear to have been cut-and-pasted
from another method).

Change-Id: I019b0ef9c75ff47fc0f2df8c95da9d090710a5a8
2013-09-18 15:18:42 +00:00
jenkins-bot c98a964d5f Merge "Fix check for preformatted when stripping whitespace" 2013-09-18 01:11:46 +00:00
Roan Kattouw d3af8b877b dm.Surface: Initialize selection at (1,1) instead of (0,0)
Code with a similar purpose was added in 568e0e5701 but got lost
when some things were moved from ve.Surface to ve.ce.Surface in
5012ed10.

Initializing the selection at (0,0) was known to cause problems before,
and since 789d0caf09 breaks editing of empty documents: typing in an
empty document begins in an inline slug, but SurfaceObserver doesn't
notice typing in an inline slug unless the ce.Surface pawns it, which
is OK because insertions in slugs are always pawned, but the pawning
logic believes the cursor to be at offset 0 where there is no slug
(it's at offset 1) and so it doesn't pawn.

Bonus: update tests and add descriptions for dm.Surface.change tests

Change-Id: Id72314d0fe650dacc7cdb842f5cea2f3bfba5145
2013-09-18 01:06:34 +00:00
jenkins-bot 973d9f999d Merge changes I899d2b3c,I0272fe38,I7ad24e7f
* changes:
  Rewrite MetaList.onTransact
  Allow replace operations to replace metadata as well
  Fix processing of double metadata replacements
2013-09-17 21:01:41 +00:00
Ed Sanders e7aed52d93 Fix check for preformatted when stripping whitespace
Previous was checking by string match to 'preformatted' which didn't
match 'mwPreformatted'. Now using node factory to see if
hasSignificantWhitespace is set.

Bug: 54083
Change-Id: I1be2d4568df7848e08074d200fb374acd508f6fa
2013-09-16 16:41:45 +01:00
C. Scott Ananian 3928ca16fd Transactions: Add trailing retainMetadata when there is trailing metadata.
Ensure that all transactions move the cursor past the trailing
metadata, if present.

Change-Id: I869c75f8cfabe1e14bb66d8e627d0b750bde46af
2013-09-11 16:02:12 -07:00
Roan Kattouw ccef625a08 Rewrite MetaList.onTransact
The previous implementation couldn't deal with transactions that
replaced both data and metadata at the same time (rather than replacing
data while moving metadata around), and extending its approach to
deal with that case would have made it much more complex.

So I rewrote the algorithm from scratch. The previous implementation
scheduled deferred moves for existing items, but immediately processed
insertions and removals. This is problematic for replacements and
maintaining the order in the binary search list. So instead, this new
implementation builds an array representing what the new item list
should be, then processes insertions, removals and moves in the correct
order to achieve that state.

It looks like the previous implementation didn't always work correctly,
which was masked because the test suite passed full=false to
assertItemsMatchMetadata(). This rewrite fixes this.

Also remove setMove/applyMove from MetaItem, because we don't need them
anymore and they're evil anyway; and add isAttached(), because the new
algorithm needs it.

Change-Id: I899d2b3c94c2cfa55823879bca95456750f64382
2013-09-11 15:29:28 -07:00
Roan Kattouw 13cd29ea62 Allow replace operations to replace metadata as well
This was already possible in the operation structure by using
removeMetadata and insertMetadata, but no transaction builders
generated transactions like that.

With this change, pushReplace() can be instructed to overwrite the
metadata with arbitrary data rather than collapsing it by passing the
optional insertMeta parameter.

Change-Id: I0272fe38e053161c738591cf0a7b447a0827d4dc
2013-09-11 15:26:12 -07:00
Roan Kattouw eb90af29e0 Fix processing of double metadata replacements
Transactions that replaced metadata twice at the same offset
(with retainMeta-replaceMeta-retainMeta-replaceMeta) were broken
because the processor for replaceMetadata didn't advance the
metadata cursor.

Change-Id: I7ad24e7ffb4c39b40ec9c347db301f8e28f3692d
2013-09-11 15:26:12 -07:00
C. Scott Ananian 1f5b78ab94 Fix off-by-one error with metadata-mutating transactions.
Add some test cases for documents with trailing metadata, and fix an
off-by-one error in the metadata-mutating transactions (since the
document metadata array is one larger than the document data array).

Change-Id: I8f049466e03ed55010dfcf0a35702536edfa7b0a
2013-09-11 10:07:20 -07:00
jenkins-bot cba2935173 Merge "Add alt attribute to core image nodes" 2013-09-10 01:43:40 +00:00
Ed Sanders dd73b873a8 Add alt attribute to core image nodes
Also update and refactor tests.

Change-Id: I96d08faed42118b713af8011c5d6befb760e65c9
2013-09-07 12:56:17 -07:00
Timo Tijhof 6843f15962 doc: Fix incorrect syntax
These are not considered errors in jsduck 4.x but are in 5.x.

Though we don't specifically want to support 5.x yet, these are
actually genuine errors that were already causing incorrect
documentation to be generated, even under 4.x (it just doesn't
warn for these under 4.x).

Not from 5.x warnings, but fixed while at it:
* Removed more redundant @method
* Added a missing @returns for ve.Range#flip
* Added newline at EOF

Change-Id: Idc22b46b082a813e8e1761403e1ebf908bf690c4
2013-09-06 20:29:18 +00:00
C. Scott Ananian 47545a5d6f Remove no-insertion metadata corner case from ve.dm.Transaction.pushReplace().
This version pushes a `replaceMetadata` operation after a `replace` to
fixup trailing metadata if there is no inserted region and the removed
region includes metadata.  This avoids a corner case where the
size of the metadata arrays inserted/removed in `Transaction.pushReplace()`
do not match the size of the data arrays inserted/removed.

We remove the now-unused `Document.getMetadataReplace()` method.

We also adjust `MetaList.onTransact()` so that it continues to work
properly when the number of metadata entries in `replace.insert` is
not the same as the number of metadata entries in `replace.remove`.

Change-Id: I1d600405b855ca1cb569853bb885b0752df47173
2013-09-06 01:07:37 +00:00
C. Scott Ananian 5830bce72c Correctly preserve metadata in Transaction.newFromUnwrap.
The `Transaction.pushReplace` method has a corner case if the removed
region has metadata and the inserted region is empty.  This works fine
unless there are two adjacent `pushReplace` operations, which can occur
in `Transaction.newFromUnwrap`.  Fix this by having `pushReplace` look
at a preceding replace and correctly merge the two operations if
possible (in particular in the tricky case where the previous case has
a zero-length insertion).  Pleasantly, this can be done without a lot of
special-casing code in `pushReplace` or `newFromUnwrap`.

Add test cases verifying the `newFromUnwrap` works correctly (both
in commit and in rollback) when there is metadata present.

Change-Id: I6cfec0d2b1823dad724422f018a3c73dc0c7f186
2013-09-06 01:06:59 +00:00
David Chan 8a2b55321c docs: Minor improvements
CODING.md
* Document the procedure for adding a new javascript class

ve.dm.MWExternalLinkAnnotation.js
ve.dm.MWInternalLinkAnnotation.js
ve.dm.ElementLinearData.js
ve.dm.LinearData.js
* Add whitespace line before preformatted code to fix a
  rendering bug

Change-Id: I54443ea3d4799328655d279f379d4ddc176c50a0
2013-09-05 23:56:59 +00:00
jenkins-bot 8dc93b6a71 Merge "Merge undefineds into undefined rather than []" 2013-09-04 22:40:17 +00:00
Roan Kattouw 247368b902 Merge undefineds into undefined rather than []
ve.dm.MetaLinearData.static.merge( [ undefined, undefined, ... ] )
returned [ [] ], make it return [ undefined ] instead. This allows
us to lazily use the merge function to create metadata without having
to worry about denormalizing undefined to [] all over the place.

Change-Id: I41fe6472c2185a6315119a546a73765966d013d3
2013-09-04 13:09:04 -07:00
jenkins-bot fc5003aa35 Merge "Collapse metadata on any removal." 2013-09-04 02:22:31 +00:00
C. Scott Ananian 7c1623d915 Cleanups for ve.dm.Transaction
Avoid making a copy of the entire data array by using `doc.data` and
`doc.data.getLength()` instead of `doc.getData()` and
`doc.getData().length`.

Get rid of some unnecessary conditionals in `Transaction.newFromWrap`: the
`tx.pushReplace()` and `tx.pushRetain()` methods will already gracefully
no-op if the replaced/retained region is empty; we don't need to add
extra control flow to handle that case.

Clarify offset math in `Transaction.newFromWrap`.

Fix a minor bug in `Transaction.newFromInsertion`: the final retain didn't
account for the length of the removed region, if `doc.fixupInsertion`
creates one.  (This usually doesn't happen.)

Remove an inaccurate TODO in `Transaction.pushReplaceMetadata()` -- it is
no longer a straight copy/paste of `Transaction.pushReplace`, so a refactor
isn't really called for.

Change-Id: I7d86a2449978365d69d4a5ed43116c2e9945475d
2013-09-04 01:41:07 +00:00
Trevor Parscal 8dfbc5baa5 Make tools generic and add fancy tool groups
Objectives:

* Got rid of mw prefixing in tools, inspectors and dialogs
* Simplify tool classes so they can be generically used as items in bars, lists and menus
* Add support for a catch-all toolbar group
* Simplify tool registration, leaning on tool classes' static name property
* Move default commands to command registry
* Move default triggers to trigger registry
* Get language tool working in standalone

Change-Id: Ic97a636f9a193374728629931b6702bee1b3416a
2013-09-03 11:27:39 -07:00
C. Scott Ananian 5095e27925 Collapse metadata on any removal.
In ve.dm.Document.getMetadataReplace(), we used to only merge metadata
if the amount removed is larger than the amount inserted.  But this
could end up putting metadata in odd positions, for example if you
have Foo[[Category:Bar]]Baz and you delete 'ooBa' and replace it with
{image}xxx{/image}, then the category ends up inside the image.

We should always merge metadata when a segment is deleted, so that it
appears outside any new structure added.

There's a weird corner case here when a segment is removed but no
insertion is made: the removed metadata then needs to get glommed onto
the next element.  We extend the insert/replace metadata array
when this happens.

Bug: 53444
Bug: 53445
Change-Id: I51d55fb370b473273f9cf152fdd0f356377d4109
2013-09-01 11:00:51 -04:00
jenkins-bot 0cdcf2d155 Merge "Always give references names. Ignore if used once." 2013-08-29 18:26:04 +00:00
Ed Sanders 52de8b76ec Always give references names. Ignore if used once.
This avoids problems when unnamed references were copy-pasted.

Knowing that key is always non-null simplifies a lot of logic
elsewhere.

Bug: 53365
Change-Id: I3a23123ae732d9583814d38dd880a0cdf691fd5d
2013-08-28 12:47:33 -07:00
Roan Kattouw 051867c3ab Actually use GeneratedContentNode in AlienNode
ce.AlienNode was mixing in GeneratedContentNode, but wasn't actually using
it properly. This commit fixes this so that AlienNode can benefit from
the <meta>/<link>/<style> stripping code in GeneratedContentNode.

Also added a getHashObject() function to dm.AlienNode to summarize the
domElements attribute because ve.getHash() chokes on it otherwise.

Change-Id: Ief3be94f9730297abe0e3c57506b81a8ff1d136d
2013-08-27 13:53:17 -07:00