Commit graph

136 commits

Author SHA1 Message Date
Catrope 5e2c421b77 Make annotating inline elements actually work
The transaction builder would step around inline content elements when
building annotation transactions. This is now fixed.

I also tweaked the processor to tolerate attempts to annotate inline
closings. This allows the builder to generate simpler transactions,
because it doesn't have to step around the closing.

Change-Id: I1e0d7f95b38bad1b35b3e125a53350d2d126a7de
2012-11-27 14:41:40 -08:00
Catrope 49963c75fd Store the data model element in the DM tree
This is cleaner than passing around the attributes separately, and it
allows us to access the annotations in dm.LeafNode as well.

Change-Id: Ie5b90988114835831cbe5cdccf63c7cd45719e31
2012-11-27 14:36:29 -08:00
Inez Korczyński a9082e6dde Only apply HTML attributes to DOM nodes that are "safe"
* Added whitelist argument to setDomAttributes which allows filtering of attributes being set
* Added prefix argument to ve.dm.Node.getAttributes to allow extracting a subset of attributes by name prefix
* Added a whitelist to ve.ce.Node which was extracted from MediaWiki's Sanitizer class
* Replaced attribute copying code with a call to setDomAttributes using the whitelist argument, passing in attributes from a call to ve.dm.Node.getAttributes using the prefix argument

Also…

* Removed comment in constructor of ve.ce.Node, documentation for properties is usually in the getters/setters, and already was in this case
* Renamed ve.setDOMAttributes to ve.setDomAttributes
* Renamed ve.getDOMAttributes to ve.getDomAttributes
* Renamed ve.getDOMText to ve.getDomText
* Renamed ve.getDOMHash to ve.getDomHash
* Updated all callers of renamed methods

Change-Id: Id556172d5d18ea431044b9d402400e1f0e67a293
2012-11-27 14:34:29 -08:00
Catrope 1bc74b0f6d Fix tests for fda2e6c1b5
Change-Id: I2725f3f775e092bafe7aa9dca71d2a9022f16db8
2012-11-26 21:39:14 +00:00
Timo Tijhof 1ba75b7ea9 The last ever mw.ext.ve jshint fixup
Change-Id: I262673214dd59e5bcaf4e0855e68728365b041fe
2012-11-26 22:36:07 +01:00
Trevor Parscal b6139ba65e Merge "(bug 42124) Store comments in the meta-linmod" 2012-11-21 22:12:41 +00:00
Trevor Parscal f825e0093a Merge "(bug 42212) Fix JS error when inserting after alien at the end" 2012-11-21 21:56:19 +00:00
Catrope bf7b243627 (bug 42121) Change markers lost for first paragraph on new page
When editing a new page, or loading an empty page into the editor, the
converter generates a paragraph so the document isn't completely empty.
This paragraph is then unwrapped on the way out, potentially destroying
change markers and generally producing strange HTML output.

Mark this paragraph with generated=empty rather than generated=wrapper,
and only unwrap it on the way out if it's still empty. This means we
cleanly round-trip empty documents (and empty list items and the like),
but if the user enters text, we create a paragraph like we're supposed
to.

Change-Id: Id0241221a67b769445676b833b5741320d99ea5f
2012-11-21 13:54:52 -08:00
Catrope 662880605c (bug 42119) Handle alienation in wrapping mode properly
When alienating in wrapping mode, we need to look at the type of tag to
decide whether to create a wrapped alienInline, or to interrupt the
paragraph for an alienBlock.

This was being done just fine for the general alienation case
(unrecognized tag), but not for the special cases (mw:unrecognized,
about groups).

* Centralize the logic for ending a wrapper in stopWrapping()
* Move the wrapping-contingent block/inline detection logic into
  createAlien()
* Simplify the terrible if statement to decide whether a future decision
  requires us to stop wrapping. Instead, detect the cases in each code
  path separately and call stopWrapping() as appropriate
* Add tests

Change-Id: I4054584ae05e7d5daa71edead3e6a6588cf5d3bb
2012-11-21 13:42:13 -08:00
Catrope 1234a702c9 (bug 42218) Add MWEntityNode
<span typeof="mw:Entity"> tags are now correctly represented in the
model, and rendered in CE. There are still issues with cursor movement
etc. in CE.

Because the prioritization mechanism for annotations vs nodes is broken
in the current "node API", I had to hack two special cases for mw:Entity
into the converter. I also had to change the converter to ignore the
children of inline nodes (this was a legitimate bug, but had never come
up before).

Change-Id: Ib9f70437c58b4ca06aa09f7272bf51d9c41b18f2
2012-11-20 16:19:55 -08:00
Catrope 3a047e0208 (bug 42124) Store comments in the meta-linmod
* Make converter generate meta nodes with 'style': 'comment'
* Handle style==='comment' in MetaBlockNode toDOM converter
* Add some comments to the meta test case
** Update other tests accordingly
* Change getDomElementSummary() to actually assert presence of comment
  nodes (specifically, all non-text child nodes)

Change-Id: Ieef9418f4c47df3541477d9420aa2ab8df6e3df1
2012-11-19 20:01:09 -08:00
Catrope a833691421 Merge "Fixing Pre-Annotations" 2012-11-20 01:11:04 +00:00
Trevor Parscal 9c22ee346a Added undo-before-apply for new link annotations
When the link inspector is used to create a new annotation, the text is annotated with the default link target derived from the selected text. Then if the inspector is used to change that value, yet another transaction is processed when the inspector is closed.

To avoid having to press undo 2x, this change makes the inspector undo it's first change before applying the changed annotation.

This change also introduces insert, remove and select content actions.

Change-Id: I3e29189158fb01336d6b053bc2a8bda2a91a0a46
2012-11-19 17:10:05 -08:00
Christian Williams 9787166bab Fixing Pre-Annotations
AnnotationAction and SurfaceFragment now use insertAnnotations.

ve.dm.Surface.test
* Removed test for annotate method (not needed anymore)

ve.dm.SurfaceFragment
* Now using getInsertionAnnotations method
* Added support for modifying insertion annotations when annotating a zero-length selection

ve.dm.Surface
* Moved in insertion annotations state from document model
* Added insertion annotation interface (enable, disable, areEnabled, get, set, add, and remove)
* Simplified handling of annotations on change
* Removed annotate method (not used anymore)

ve.dm.Document
* Removed insertion annotations (moved it to surface model)

ve.ce.Surface
* Cleaned up handleInsertion and changed it to use the insertion annotations interface on the surface model

ve.AnnotationAction
* Moved insertion annotation handling out of here since it's now included in the surface fragment

Change-Id: I047d656acf7fa1c63f726ca2b0801e1476f84f96
2012-11-19 17:09:08 -08:00
Catrope 79e4b139fb (bug 42212) Fix JS error when inserting after alien at the end
TransactionProcessor was using parentOuterRange without checking whether
it was present, so it was exploding for indexInNode results.

Now checking for parentOuterRange presence, and falling back to
nodeOuterRange when missing.

This fix causes inconsistencies with zero-length text nodes. We should
fix these eventually, but for now I've just made the unit tests
tolerant of zero-length-text-node deviations.

Change-Id: Id9eadd57a0d5fcbaf009c0781da0a03928aebb31
2012-11-19 14:35:30 -08:00
Trevor Parscal 2c8411eb62 (bug 41947) Propagate change markers when unwrapping generated nodes
Editing the text of a list item results in a change marker on the
paragraph within that list item. However, that paragraph usually isn't
present in the HTML, so the converter unwraps it when converting back to
HTML, and the change markers are lost. Instead, transfer the change
markers to the <li>.

Change-Id: Id675075d19c08d69bc8e990174841dc393b749fc
2012-11-16 15:39:35 -08:00
Catrope 29a0c38e05 Add ce.AlienNode to tests
Change-Id: I591501067732ae36c89da340398ad35e8d4b6e3d
2012-11-16 15:35:09 -08:00
Catrope d4ea93b872 Add basic support for about groups
About groups are HTML structures like the following:
<div about="#mwt1">....</div>
<span about="#mwt1">...</span>
<div about="#mwt1">...</div>
When about groups are alienated, they are now merged into one alien
node, rather than producing a separate alien node for each sibling.

This is very basic about group handling, because it only works for
groups of directly adjacent siblings (text nodes are permitted in
between, but nothing else) assumes all about groups are aliens (which
is currently true).

* Before processing an element in the DOM->data converter, perform about
  grouping on its children. This temporarily wraps about groups in
  <div data-ve-aboutgroup="value of about attribute">
* Extended createAlien() to handle single nodes as well as wrappers
  holding multiple nodes.
* In the data->DOM converter, temporarily wrap multi-node aliens in
  <div data-ve-multi-child-alien-wrapper="true"> . This makes the rest
  of the algorithm easier.

Change-Id: I2df5f62bc222b570fc11a89fe43d353f8363ead8
2012-11-07 18:13:50 -08:00
Catrope 03827764a7 Merge "Refactored commands into a registry" 2012-11-07 23:52:59 +00:00
Trevor Parscal 443c5438ab Refactored commands into a registry
* Now ve.Factory inherits from the more general ve.Registry
* New class ve.CommandRegistry
* Refactored setupToolbar and command setup code into setupComands

Change-Id: Ic548e5de95b77889727362d3e66d7be83c12a603
2012-11-07 15:52:30 -08:00
Catrope 3d76ce60fe Merge "Replaced command factory with new command class" 2012-11-07 23:51:19 +00:00
Trevor Parscal ab57bed7da Replaced command factory with new command class
The port of mousetrap wasn't really what we needed. This is much simpler, matches the rest of our code, and does exactly what we need.

Change-Id: I67f413e097fc2d4078336edb14dd9440e771f196
2012-11-07 15:47:03 -08:00
Trevor Parscal f4a674e2c5 Merge "Flag pre nodes as having significant whitespace" 2012-11-07 22:12:45 +00:00
Trevor Parscal 273c6ac7bd Made commandFactory tests not break anymore
Sequences that were scheduled directly after each other, such as "a b c" and "1 2 3" would end up overlapping sometimes, producing "a b 1 c 2 3" which failed to trigger the correct commands.

Change-Id: I27bb60e856e9d692a21e1587dc227f8aeb5fcf4e
2012-11-07 13:56:53 -08:00
Catrope 1f01100eb9 Flag pre nodes as having significant whitespace
This causes the converter not to strip inner whitespace in them, and
causes CE to suppress the whitespace mangling logic that is normally
applied (↵ for newlines, ➞ for tabs, alternating &nbsp;s for spaces).

Change-Id: I738a750c91a4ca4836c485e282865bb7525bf30a
2012-11-07 12:10:58 -08:00
Trevor Parscal 6082953562 Made jquery.multiSuggest it's own RL module
* Also updated resource paths in tests and demo pages

Change-Id: Ic21b678f91ed8d1fe1c7ac2a53d89270aacf7c26
2012-11-06 16:37:01 -08:00
Catrope 04a999f991 Add change marking for Parsoid's benefit
* Add map of change markers per offset to Transaction
* Map is populated by TransactionProcessor
* Markers are reversed on rollback
* Removals aren't marked, Parsoid can detect these using DSR
  discontinuities

Change-Id: I2290886ab411c6ad6162044ed85c091313613e51
2012-11-06 10:11:11 -08:00
Catrope 857535b63f Introduce meta-linmod
* ve.dm.Converter still generates metaInline/metaBlock elements as
  before, it's not affected by this change
* ve.dm.Document constructor splits its input into "real" data and
  metadata
** Metadata is stored in this.metadata (the meta-linmod) as a sparse
   array of arrays, with an element for each offset in this.data
** this.data itself does not contain the metadata
** This means the node tree also doesn't contain the metadata
** Which means CE doesn't know about it at all
* All splice operations on the linear model are sent through
  ve.dm.Document.spliceData(), which performs the splice and syncs the
  meta-linmod
** Metadata in the removed range is reaped and added to the metadata for
   the offset immediately following the removal
* ve.dm.Document.getFullData() splices the linmod and meta-linmod back
  into each other; this "full data" is then fed back to ve.dm.Converter

Change-Id: Ief6dfd5b59cc13a8457993ed85c725413029c4fb
2012-11-02 19:06:49 -07:00
Catrope 6ef9fa78ff Fix ve.batchSplice() to behave in line with docs
* Actually return the spliced data like the docs claim we do
* Remove false claim that offset can be negative
* Add that data=[] && remove=0 is invalid; native splice() doesn't allow
  this, and there is a case where we call native splice() directly
* Add tests

Change-Id: I90e77c1b22ea1c36cb61e89ea47831885a0b1cb9
2012-10-30 10:05:49 -07:00
Trevor Parscal 82a15d66fc Merge "Fix copyObject/copyArray behavior with null values" 2012-10-30 16:59:02 +00:00
Catrope d30096ebd3 Fix copyObject/copyArray behavior with null values
Previously copyObject and copyArray would silently drop null values,
which is bad, especially considering we have example data for meta nodes
that has { 'key': null } somewhere.

Also added a test case that failed prior to this change.

Change-Id: I4f233cce041fbf38f701c494f1f78ac3d8535d88
2012-10-29 19:45:25 -07:00
Catrope 6df508cf8b Add InspectorFactory to tests
Tests were completely broken because the link inspector threw a JS error
when trying to register itself with the nonexistent inspector factory.

Change-Id: I8a47222f0a5a37348262ed939b37fbc47d14e222
2012-10-29 18:36:54 -07:00
Timo Tijhof 4cc2101ffd Test: Enforce # of expected assertions.
Change-Id: I041c792d1841f69677f8c7d38f67108475a0afc9
2012-10-25 22:06:07 +02:00
Krinkle dae235111c Merge "Fix JS error in ve.setProp()" 2012-10-25 19:55:16 +00:00
Catrope 84efb81e7e Fix JS error in ve.setProp()
Attempting to descend into a string or number would cause a JS error,
because we would attempt to create prop[arguments[i]] as an empty object
(which is ignored), then try to descend into it (which blows up because
it's undefined, even though we've just set it). Guard against this by
explicitly checking for non-object-ness.

Change-Id: Ie65550baaae0ab88476c9a1ff40cc136090740a0
2012-10-25 21:54:45 +02:00
Trevor Parscal 4e87a7a79b Fix number of doc sync tests
Follow up to I84f0368b1d7b601ed0766806607152dc97f34603

Change-Id: Ic1e9de1e755b8966b6e964b01b00f4d99d27245e
2012-10-25 11:06:12 -07:00
Catrope 2c1683ecff DocumentSynchronizer fix and cleanup
* Adjust the range in the annotation synchronizer, otherwise we emit
  events for the wrong node
* Expanded test suite to the point where it was able to catch the bug
  caused by not adjusting annotated ranges
* Removed selection.length === 0 check, no longer needed because
  selectNodes() now throws an exception in this case
* Added a FIXME comment about duplicate update events that occur when
  length adjustments are combined with something else
* Add a few more comments

Change-Id: I84f0368b1d7b601ed0766806607152dc97f34603
2012-10-25 11:02:58 -07:00
Trevor Parscal 8e7facf222 Merge "Guard transactions against double commit/rollback" 2012-10-25 17:55:56 +00:00
Trevor Parscal 9c51bb2d7b Merge "No longer copy data in ve.dm.Document constructor" 2012-10-25 17:54:23 +00:00
Catrope 29cff8c105 Guard transactions against double commit/rollback
* Store the applied state in the Transaction
* Store the Transaction in the TransactionProcessor (previously, only
  its operations were stored)
* Have commit() and rollback() throw exceptions when passed transactions
  with the wrong applied state
* Add tests for this behavior

Change-Id: I27b7a96fdf4d3555d78f64c05a03702ea560c802
2012-10-24 17:47:41 -07:00
Catrope 43f1612324 No longer copy data in ve.dm.Document constructor
The data array is now taken by reference, and the caller must perform
any copying required.

Changed tests to make a deep copy of shared data sets (mostly
ve.dm.example) before passing them to ve.dm.Document().

Change-Id: Iedc64f9fd9cd689640de9a19379cf5f3db94a2bb
2012-10-24 17:32:35 -07:00
Catrope aa2836aa6e Remove 'internal' property from DM nodes
There's no use case for keeping a deep copy of the 'internal' property
in the node tree, and it was breaking some of my new tests concerning
change markers. We could keep internal data in the node tree if we
wanted to, but to be correct we'd have to synchronize every time we
changed it, which is a pain.

Change-Id: I024de1ff8b6b6154da82c103c4bb21db8ff2ec14
2012-10-24 16:47:14 -07:00
Trevor Parscal b8dc697f17 Merge "ve.ui.CommandFactory: Initial implementation" 2012-10-24 18:48:40 +00:00
Timo Tijhof 34cbc729db ve.ui.CommandFactory: Initial implementation
Based on https://github.com/ccampbell/mousetrap.

Cleaned up to fit our coding standards, pass JSHint, and assume
jQuery's fixes where possible (e.g. no need for an addEvent
utility, no need for filling e.target, e.which, etc. cross-browser
which jQuery.Event already does).

Initially all were local functions in the constructor, but to
allow some customisations in subclasses moved various methods
to the prototype instead and marked with @private.

Really, only .register() must be called from the outside. The rest
assumes normalisation etc. or might break things if called
incorrectly.

Change-Id: Ic69a3c70759052094aefbeab623d885f8b118e14
2012-10-24 17:48:16 +00:00
Timo Tijhof 37f3a288ec Standards: Fix global variables and pass JSHint.
Follows-up:
* IK 714e29d30f
* IK 3b8a5ae4d5
* IK 72eb2825e5
* RK 7fe7182f43
* ...

Change-Id: I671f08e4899bfb9508cef272190ec72721a0af9a
2012-10-23 00:53:48 +02:00
Catrope 61b6c1ce86 Add parentOuterRange to selectNodes() output
This is the outer range of the parent of the node, if known. We'll need
this for change marking: when resizing a text node, for instance, we
need to mark its containing parent. This way we get the containing
parent's element's offset for free (selectNodes already tracks it in
currentFrame) rather than having to compute it with another traversal.

Change-Id: Ia335d8080ea9d414ab9f89b943e2ea0cd11d7df3
2012-10-19 15:28:26 -07:00
Catrope ef513244be Do reference comparison in selectNodes() tests and fix test data
Some tests were using the wrong node in the expected data, but because
only the summaries were compared, this would succeeed as long as the
type and length were equal (and paragraphs of length 1 are quite common
in our test data). Fixed equalNodeSelection() to compare each node by
reference as well as comparing the summaries. If one of the equality
tests fails, the summaries will still be displayed as expected/actual
data (even though they might be equal), and the message will  have
"(reference equality for selection[3].node)" appended to it.

This change broke the tests because a few test cases had bad data, fixed
those in this commit as well.

Change-Id: Iab420cf29d47f7368c8a9ce79f6309efae75685c
2012-10-19 13:56:46 -07:00
Catrope cc9c530690 Add ve.setProp()
Change-Id: I6f932917f8e6321e9c415900b70404406af96d5c
2012-10-19 11:22:12 -07:00
Catrope 937607893c Fix selectNodes() bug reported by Inez
For <p>1<br/>2</p>, selectNodes([2,2]) correctly returned the end of the
first text node, but selectNodes([4,4]) returned index 2 in the
paragraph (i.e. between the break node and the second text node). The
correct behavior is to return the start of the second text node, i.e.
the mirror image of the behavior for [2,2].

Fixed this by applying the startBetween/endBetween logic only if the
relevant adjacent node is wrapped (or if it's missing). In the code,
this is expressed as !(adjacent node present && adjacent node wrapped).

Change-Id: Ie3b7fdf1de38ee253a798a7a73bc89734f4ca4fa
2012-10-17 16:09:49 -07:00
Catrope 84e598953a Wrap inline elements properly
The HTML "1<br/>2" was being converted to a linmod that looked like
"<p>1</p><br></br><p>2</p>". This commit fixes the wrapping logic such
that the result is "<p>1<br></br>2</p>" instead. In general, inline
nodes (content nodes) should not interrupt the wrapping, but block nodes
should.

This creates a problem for alien nodes: normally, we determine whether an
alien node is a block alien or an inline alien based on context, but if
we're in wrapping mode we're unsure of the context. We can't tell the
difference between "1<tt>Foo</tt>2" (should be wrapped as one, because
tt is inline) and "1<figure></figure>2" (1 and 2 should be wrapped
separately, because figure is block) using context alone, so in these
cases (and ONLY in these cases) we look up whether the HTML tag in
question is an inline tag or a block tag and use that to decide.

Change-Id: I75e7f3da387dd401d9b93e09a21751951eccbb83
2012-10-17 13:50:29 -07:00