Commit graph

324 commits

Author SHA1 Message Date
Catrope 1bc74b0f6d Fix tests for fda2e6c1b5
Change-Id: I2725f3f775e092bafe7aa9dca71d2a9022f16db8
2012-11-26 21:39:14 +00: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
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 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 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
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
Timo Tijhof 4cc2101ffd Test: Enforce # of expected assertions.
Change-Id: I041c792d1841f69677f8c7d38f67108475a0afc9
2012-10-25 22:06:07 +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
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 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
Trevor Parscal 4fbf7308f7 Reversed the default value of autoSelect in surface fragments
Arguments with default truthy arguments are evil

Change-Id: I3fb972af1b8f52837497950281c537fe09eb7975
2012-10-12 17:34:15 -07:00
Catrope 405581f6b8 New annotation API: update tests
Change-Id: I301a1d89c33dd68197d629d187a9a5be8cbf3852
2012-10-12 15:07:28 -07:00
Catrope 7fe7182f43 New annotation API: Annotation and AnnotationFactory classes
Fleshes out ve.dm.Annotation to a class. Annotations in the linear model
will be instances of a subclass of ve.dm.Annotation. Annotations are
defined by subclassing ve.dm.Annotation and registering this subclass
with ve.dm.AnnotationFactory.

ve.dm.AnnotationFactory keeps track of which annotation classes are known,
and has code to match an HTML element to an annotation class, for use in
the converter.

Change-Id: I68802bdb8736ced1f9e04ee49c623944b448141c
2012-10-12 15:07:02 -07:00
Catrope 9ca5da44ee Merge "Revert "No longer create zero-length text nodes"" 2012-10-12 18:04:22 +00:00
Catrope 0a6a2c7cd8 Revert "No longer create zero-length text nodes"
Inez asked for this to be merged but now says it's broken

This reverts commit 7702ec10dc
2012-10-12 18:04:15 +00:00
Trevor Parscal b3a90966f3 Merge "No longer create zero-length text nodes" 2012-10-11 19:42:52 +00:00
Christian Williams f2d08f913b Added reversed boolean for translateOffset
Previously, Undo used a transaction's lengthDifference to calculate the selection to display after the transaction was undone. Now, translateOffset with the reversed boolean set to true will properly translate the inverse of a transaction's selection change. Fixes bug #40538

Change-Id: I110bc0cbb5824547842efd391b9f2948b037b758
2012-10-10 14:59:30 -07:00
Catrope 7702ec10dc No longer create zero-length text nodes
We were populating empty content nodes with zero-length text nodes to
make round-trip tests in the test suite work (otherwise blanking a
paragraph leaves behind a zero-length text node whereas creating an
empty paragraph does not), but the empty nodes are causing problems in
CE apparently.

* Do not create empty text nodes when constructing a node tree
* Be more careful with text-only replacements:
** Don't resize a text node to zero, remove it instead
** There may not be a text node to resize at all, build it in that case
** Switch nodeRange to nodeOuterRange, this was probably broken before

Tests:
* Change test case for zero-length text node to assert that there is
  *no* zero-length text node :)
* Remove a test case concerning an empty text node from the
  ve.ce.TextNode suite

Change-Id: Ie677457f2f0a7823a517ba3077b844ef52a20fcc
2012-10-10 14:48:47 -07:00
Krinkle a5a745de69 Merge "ve.dm.SurfaceFragment: Implement wrapNodes and wrapAllNodes" 2012-09-24 19:11:35 +00:00
Trevor Parscal eabe5e6f61 ve.dm.SurfaceFragment: Implement wrapNodes and wrapAllNodes
Change-Id: I378f0aad0286a6c90adeb4602a57d6617154e8b6
2012-09-24 21:11:16 +02:00
Trevor Parscal daa7e76807 Merge "Add a node type for meta nodes" 2012-09-18 18:15:46 +00:00
Timo Tijhof ab7d6bf082 Documentation & clean up
* Commands for Sublime:

  Find*: "(\* @[a-z]+) ([^{].*) \{(.*)\}"
  Replace: "$1 {$3} $2"

  Save all && Close all

  Find: " function("
  Replace: " function ("

  Save all && Close all

  Find: "Intialization"
  Replace: "Initialization"

  Save all && Close all

* Consistent use of types (documented in CODING.rm):
  - Merged {Integer} into {Number}.
  - Merged {DOM Node} into {DOMElement}.

* Remove work-around /*jshint newcap: false */ from ve.js
  Calling Object() as a function to to use the internal
  toObject no longer throws a newcap warning in JSHint.
  It only does that normal functions now .

  (e.g. var a = Cap(); or var a = new uncap();)

* Add missing annotations (@static, @method, ..).

* Remove unused variables

* Remove null-assignments to variables that should just be
  undefined. There's a few variables explicitly set to null
  whereas they are set a few lines under and not used otherwise
  (e.g. 'tx' in ve.ce.Surface.prototype.onPaste)

Change-Id: I0721a08f8ecd93c25595aedaa1aadb0e08b83799
2012-09-17 16:02:52 +02:00
Catrope 7b96fbe3d2 Add a node type for meta nodes
This node type represents <meta> or <link> (transparently, based on the
style attribute). I had to make two node types for this and hack the
toData conversion code directly into ve.dm.Converter, because we don't
have native support for node types that can be both inline and block.
(We should add this in the node API rewrite.)

The CE implementation renders a placeholder (with the same styles as an
alien node) right now. I'm not sure how nice that is, but it's better
than rendering raw <meta>/<link> tags.

This whole thing is a total pile of hacks to make VE deal with
<meta>/<link> tags until we have a proper node types API.

Change-Id: Id6783fcfc35a896db088ff424ff9faaabcaff716
2012-09-10 15:35:30 -07:00
Catrope c6cb537f1a Fix bugs in whitespace preservation for aliens
This was broken in three different ways:
* On the way in, we were applying whitespace to an array of elements
  rather than the actual element, so the whitespace wasn't stored.
* Whitespace processing on the way out was skipped for aliens because
  they had their own code path. Refactored this so alien openings and
  regular openings share much more code, including whitespace output.
* Somewhat unrelatedly, innerPost output was broken for paragraphs
  containing inline elements, because the inline elements' processing
  polluted lastOuterPost. Discovered this because my test with inline
  aliens also happened to be the first test of whitespace preservation
  in paragraphs with inline content elements. Fixed by explicitly
  skipping content nodes when outputting whitespace.

Fixed these issues and added a test case.

Change-Id: I8edb61a008e60ace886b1a841b3417682ec39c32
2012-09-07 15:17:28 -07:00
Catrope 74ed8e8766 Rename ve_foo_bar back to VeFooBar per discussion
Change-Id: Ibf6d4f08c4761727b2e3952a76e474c8221b38f9
2012-09-06 16:15:55 -07:00
Trevor Parscal e3375e8f23 Merge "Fix global scope leakage in the Transaction tests" 2012-09-06 22:51:55 +00:00
Catrope d43fffc2d2 Fix global scope leakage in the Transaction tests
Change-Id: I9283acb6179d0be84dadca8ad4e58d5cdd6e3bae
2012-09-06 15:47:33 -07:00
Catrope ef27a4b2b1 Fix order-of-annotation tests for hrefPrefix
Change-Id: I9fffad6cd0b186a070fec34fd0f827a5875a5d67
2012-09-06 15:40:41 -07:00
Timo Tijhof b1d9c83b5d Object management: Object create/inherit/clone utilities
* For the most common case:
  - replace ve.extendClass with ve.inheritClass (chose slightly
    different names to detect usage of the old/new one, and I
    like 'inherit' better).
  - move it up to below the constructor, see doc block for why.

* Cases where more than 2 arguments were passed to
  ve.extendClass are handled differently depending on the case.

  In case of a longer inheritance tree, the other arguments
  could be omitted (like in "ve.ce.FooBar, ve.FooBar,
  ve.Bar". ve.ce.FooBar only needs to inherit from ve.FooBar,
  because ve.ce.FooBar inherits from ve.Bar).

  In the case of where it previously had two mixins with
  ve.extendClass(), either one becomes inheritClass and one
  a mixin, both to mixinClass().

  No visible changes should come from this commit as the
  instances still all have the same visible properties in the
  end. No more or less than before.

* Misc.:
 - Be consistent in calling parent constructors in the
   same order as the inheritance.
 - Add missing @extends and @param documentation.
 - Replace invalid {Integer} type hint with {Number}.
 - Consistent doc comments order:
   @class, @abstract, @constructor, @extends, @params.
 - Fix indentation errors
   A fairly common mistake was a superfluous space before the
   identifier on the assignment line directly below the
   documentation comment.
   $ ack "^ [^*]" --js modules/ve
 - Typo "Inhertiance" -> "Inheritance".
 - Replacing the other confusing comment "Inheritance" (inside
   the constructor) with "Parent constructor".
 - Add missing @abstract for ve.ui.Tool.
 - Corrected ve.FormatDropdownTool to ve.ui.FormatDropdownTool.js
 - Add function names to all @constructor functions. Now that we
   have inheritance it is important and useful to have these
   functions not be anonymous.

   Example of debug shot: http://cl.ly/image/1j3c160w3D45

   Makes the difference between

   < documentNode;
   > ve_dm_DocumentNode
     ...
     : ve_dm_BranchNode
       ...
       : ve_dm_Node
         ...
         : ve_dm_Node
           ...
           : Object
             ...

   without names (current situation):

   < documentNode;
   > Object
     ...
     : Object
       ...
       : Object
         ...
         : Object
           ...
           : Object
             ...

   though before this commit, it really looks like this
   (flattened since ve.extendClass really did a mixin):

   < documentNode;
   > Object
     ...
     ...
     ...

   Pattern in Sublime (case-sensitive) to find nameless
   constructor functions:
   "^ve\..*\.([A-Z])([^\.]+) = function \("

Change-Id: Iab763954fb8cf375900d7a9a92dec1c755d5407e
2012-09-06 15:29:31 -07:00
Catrope 742a97b410 Add tests asserting ordering of annotations works
Change-Id: Iada85b95914a8728ca809ebab9a42723c1e7a02c
2012-09-06 14:43:13 -07:00
Catrope 5e7c14c868 Manage annotations in ve.AnnotationSet object
Introduced the ve.AnnotationSet class to manage sets of annotations. This
is a generalization of ve.OrderedHashSet, a class that manages a set
using an array and an object keyed by hash.

Converted everything that stores, tracks or passes around annotations to
use ve.AnnotationSet. In particular, this means the linear model now
contains AnnotationSets instead of hash-keyed objects.

This allows us to maintain the order of annotations in the linear model,
and will help fix bugs with annotation ordering and splitting.

Change-Id: I50975b0a95f4cc33017a0b59fdede9ed1eff0124
2012-09-06 14:39:38 -07:00
Timo Tijhof 71020f7175 Remainder JSHint fixes on modules/ve/*
[jshint]
ce/ve.ce.Surface.js: line 670, col 9, Too many var statements.
ce/ve.ce.Surface.js: line 695, col 6, Missing semicolon.
ce/ve.ce.Surface.js: line 726, col 22, Expected '===' and instead saw '=='.
ce/ve.ce.Surface.js: line 726, col 41, Expected '===' and instead saw '=='.
ce/ve.ce.Surface.js: line 733, col 13, Too many var statements.
ce/ve.ce.Surface.js: line 734, col 24, Expected '===' and instead saw '=='.
ce/ve.ce.Surface.js: line 1013, col 13, Too many var statements.
ce/ve.ce.Surface.js: line 1019, col 17, Too many var statements.
ce/ve.ce.Surface.js: line 1023, col 18, Too many ar statements.
ce/ve.ce.Surface.js: line 1027, col 13, Too many var statements.
dm/annotations/ve.dm.LinkAnnotation.js: line 70, col 52, Insecure '.'.
dm/ve.dm.Converter.js: line 383, col 29, Empty block.
dm/ve.dm.Converter.js: line 423, col 33, Empty block.

Commands:
 * jshint .
 * ack '(if|else|function|switch|for|while)\('
 * Sublime Text 2:
   Find(*): (if|else|function|switch|for|while)\(
   Replace: $1 (
 * ack '  ' -Q # double spaces, except in certain comments

Change-Id: I8e34bf2924bc8688fdf8acef08bbc4f6707e93be
2012-09-05 13:45:09 -07:00
Timo Tijhof 24590a9702 ve.dm.Converter.test: Add hrefPrefix (follows-up I33eab356)
* Tests were failing since I33eab356.

Change-Id: I1fed0e3e21ea379a583ccd4f611212940264d106
2012-09-03 03:07:24 +02:00
Trevor Parscal f26ae1662b Added tests for removeContent and insertContent
Also fixed the arguments given to insertContent and double-translating the range

Change-Id: I7cb7dcfcee1c88f2052c63e31a0ed37eaaf645ab
2012-08-30 16:55:49 -07:00
Catrope 4fbabd16dd Make translateRange() map the offset immediately before an insertion to after the insertion rather than before
This will cause ve.dm.SurfaceFragment.prototype.insertContent() to place
the selection after the insertion as well.

Change-Id: Ifa7e627daceb90408422eb58c110d475f34ba1e2
2012-08-30 15:26:43 -07:00
Catrope 452c759914 Preserve whitespace between elements
This commit fully utilizes all four positions in the internal.whitespace
array. Outer whitespace is now preserved as well, and is duplicated
either in the adjacent sibling (one node's outerPost is the next
sibling's outerPre) or in the parent (a branch node's innerPre is its
first child's outerPre, and its innerPost is its last child's
outerPost). Before restoring saved whitespace, we check that these two
agree with each other, and if they disagree we assume the user has been
moving stuff around and don't restore any whitespace in that spot. The
whitespace at the very beginning and the very end of the document (i.e.
the first node's outerPre and the last node's outerPost) isn't
duplicated anywhere, nor is inner whitespace in content nodes.

The basic outline of the implementation is:
* When we encounter whitespace, strip it and store it in the previous
  node's outerPost. Also store it in nextWhitespace so we can put it in
  the next node's outerPre once we encounter that node.
* When we encounter whitespace in wrapped bare text, we don't know in
  advance if it's gonna be succeeded by more non-whitespace (in which
  case it needs to be output verbatim), or not (in which case it's
  leading whitespace and needs to be stripped and stored). The fact that
  annotations are nodes in HTML makes this trickier. So we write the
  whitespace to the temporary linmod and store it in wrappedWhitespace,
  then if it turns out to be trailing whitespace we take it back out of
  the data array and record it the usual way.
* Because text nodes can contain any combination of leading whitespace
  actual text and trailing whitespace, and because we may or may not
  already have opened a wrapping paragraph, there are a lot of different
  combinations to handle. We handle all of them but the resulting code
  is pretty dense and verbose.

More low-level list of changes:

In getDataFromDom():
* Added helper function addWhitespace() for storing whitespace for an
  element
* Added helper function processNextWhitespace() for processing any
  whitespace passed on from the previous node via the nextWhitespace var
* Rename paragraph to wrappingParagraph. Make wrapping default to
  alreadyWrapped so we can simplify wrapping||alreadyWrapped and
  !wrapping&&!alreadyWrapped. Add wrappingIsOurs to track whether the
  wrapping originated in this recursion level (needed for deciding when
  to close the wrapper).
* Add prevElement to track the previous element so we can propagate
  whitespace to it, and nextWhitespace so we can propagate whitespace to
  the next element.
* Remove previous newline stripping hacks
* Integrate the logic for wrapping bare content with the outer
  whitespace preservation code
* Remove wrapperElement, no longer needed because we have a dedicated
  variable for the wrapping paragraph now and what was previously inner
  whitespace preservation for wrapper paragraphs is now covered by the
  outer whitespace preservation code.

In getDomFromData():
* Reinsert whitespace where appropriate
** outerPre is inserted when opening the element
** This covers outerPost as well except for the last child's outerPost,
   which is handled as the parent's innerPost when closing the parent.
** innerPre and innerPost are inserted when closing the element. Care is
   taken not to insert these if they're duplicates of something else.
* Propagate each node's outerPost to the next node (either the next
  sibling or the parent) using parentDomElement.lastOuterPost. We can't
  get this using .lastChild because we will have destroyed that child's
  .veInternal by then, and we can't tell whether a node will be its
  parent's last child when we process it (all other processing,
  including first child handling is done when processing the node itself,
  but this cannot be).
* Special handling is needed for the last node's outerPost, which ends
  up in the container's .lastOuterPost property.

Tests:
* Allow .html to be null in data<->DOM converter tests. This indicates
  that the test is a one-way data->DOM test, not a DOM->data->DOM
  round-trip test. The data will be converted to HTML and checked
  against .normalizedHtml
* Update existing tests as needed
* Add tests for outer whitespace preservation and storage
* Add test for squashing of whitespace in case of disagreement (this
  requires .html=null)

Change-Id: I4db4fe372a421182e80a2535657af7784ff15f95
2012-08-23 19:08:00 -07:00
Catrope 3ed257a5d6 Also strip ./ in addition to ../
This is necessary because Parsoid is now prefixing all hrefs with ./

Change-Id: I18c4b7470cfa1dd174e25cc921bf7d4daf5ffc55
2012-08-23 14:29:43 -07:00
Catrope dfd78cb121 Remove references to data-mw-gc
data-mw-gc is ancient and unused. We do need to detect and alienate
generated nodes, but that is now based on RDFa types. Removing the
data-mw-gc stuff for now because it doesn't work anyway, will replace it
with proper detection later.

Replaced instances of data-mw-gc in the test suite with unregistered
node types.

Change-Id: If3f5898d382a436fa57929013264c53af5e840ba
2012-08-20 17:44:55 -07:00
Catrope f4459b589b Merge "Added some unit tests for ve.dm.SurfaceFragment" 2012-08-17 20:16:00 +00:00
Catrope 41b5244a46 Merge "There's no such thing as ghosts, just sneaky array references" 2012-08-17 18:27:29 +00:00
Trevor Parscal 1e2502a726 Added some unit tests for ve.dm.SurfaceFragment
Also:
* Made a fragment with a null range become a null fragment
* Fixed incorrect order of arguments for binding a handler to transact event
* Added getters for surface, document and range
* Fixed several instances of passing a document instead of a surface into the constructor of a new surface fragment
* Fixed closest mode in expandRange - needed to check if parent existed before checking for it's type
* Fixed uses of ve.Transaction (doesn't exist) that were supposed to be uses of ve.dm.Transaction (does exist)

Change-Id: Ide13d9d2d1637399188c98c2e8b6e0826caeecc4
2012-08-17 10:48:40 -07:00
Trevor Parscal 264b97df7f There's no such thing as ghosts, just sneaky array references
When a document is created, it should take it upon itself to make sure it has a new reference to the data using slice, not place this on the caller. Callers that do not use slice will often find strange and mysterious things going on and not know why. The real reason is that multiple documents sharing a reference to the same data array leads to seriously messed up behavior.

Change-Id: Ic4e25fcd9bf3f41a805003520a8f38e2768f5dbf
2012-08-17 10:37:28 -07:00
Catrope 7319038ed6 Strip generated <p> tags in dataToDom
domToData wraps bare content in paragraph elements, which were then
converted to <p> tags by domToData. With this fix, HTML with "missing"
<p> tags actually round-trips through the editor correctly now, rather
than having <p> tags added wherever VE believes they should exist.

* Mark generated paragraph elements with .internal.generated = 'wrapper'
** This signifies the wrapper was generated but its contents were not,
   so the right thing to do when converting back to HTML is to remove
   the wrapper and keep the contents. We might want to use other values
   of generated in the future.
* Unwrap nodes with generated=wrapper when converting to HTML

Tests:
* Add 'generated': 'wrapper' as appropriate. Only affects 1 test
* Remove 'normalizedHtml' for this test because it is no longer needed
** Need to keep 'normalizedHtml' for now because we normalize hrefs
* Eventually the main example should test bare content, but that
  requires touching a lot of stuff. The main example could use some
  beefing up anyway.

Change-Id: I277ad5fe3f64e07c1bbf49007d6bbaecc90b7466
2012-08-16 16:09:28 -07:00
Catrope ce60b54d33 Rename fringeWhitespace to internal
This allows us to put other internal data in there in the future. Also
passing it through the Node constructor properly now.

* ve.dm.Node
** Rename fringeWhitespace property to internal
** Add internal parameter to constructor
** Remove setFringeWhitespace()
* Increase the number of parameters passed through by ve.Factory to 3
* Pass through .internal from linmod to nodeFactory in ve.dm.Document
* ve.dm.Converter
** Rename .fringeWhitespace to .internal.whitespace and make it an array
** Store a temporary reference to .internal in domElement.veInternal
* Add internal to all node constructors except TextNode

Tests:
* Update for fringeWhitespace->internal rename
* Add third parameter to ve.Factory tests
* Add .internal to getNodeTreeSummary

Change-Id: If20c0bb78fee3efa55f72e51e7fc261283358de7
2012-08-16 15:14:01 -07:00
Trevor Parscal c8b4a28936 Added key-sorting to make hashes referentially transparent
To do this, we are using the replacer callback of JSON.stringify, which is supported by all of our target browsers including IE8. We are also leaning on Object.keys and Array.reduce, the latter of which required adding a new fallback implementation for some browsers which do not support it yet.

Change-Id: Ifa285ca3da4d94d962464f09414591532bbea79c
2012-08-15 11:14:44 -07:00
Catrope 6f8307d5d2 Update LinkAnnotation for Parsoid href changes
Because the Parsoid prefix format changed from /mw:Foo to /mw/Foo , the
href format for internal links has changed from "/Foo" to "Foo". So the
href is now simply the title, except that it may be preceded by one or
more "../" if the title of the page we're on contains a '/'.

So instead of stripping the leading slash from internal link hrefs and
putting it back on the way out, only strip any leading "../"s and dump
the titles directly into the hrefs on the way out.

Also update the link test case for this, and add a test case for the ../
stripping.

Change-Id: I3e0bdde20f22cda34eb45fc351de5e780419b6a2
2012-08-14 11:03:37 -07:00
Catrope 70fa9c8aeb Recognize annotation types with multiple slashes correctly
Annotation types with more than one slash such as 'link/ExtLink/URL'
weren't being processed correctly because .split( '/', 2 ) throws away
everything after the second slash. Instead, don't pass a limit to
.split(); the code for reconstructing a slash-separated string from
multiple components was already in place.

Also add test cases for URL links and numbered links.

(Do you like the lines-of-code to lines-of-test ratio in this commit,
Trevor? ;) )

Change-Id: I7add87396447a01b1c23a4f9bfd63d2e8fd861ce
2012-08-13 18:22:31 -07:00
Catrope 63e6702c52 Strip and preserve inner leading&trailing whitespace in the linear model
This makes things like
== Foo ==
* Bar
render without the leading and trailing spaces, while still
round-tripping those spaces.

* Added a .fringeWhitespace property to the linear model and ve.dm.Node
** Object containing innerPre, innerPost, outerPre, outerPost
** Only inner* are used right now, outer* are planned for future use
** Like .attributes , it's suppressed if it's an empty object
* In getDataFromDom():
** Store the stripped whitespace in .fringeWhitespace
** Move emptiness check up: empty elements with .fringeWhitespace have
   to be preserved
** Move paragraph wrapping up: .fringeWhitespace has to be applied to
   the generated paragraph, not its parent
** Add wrapperElement to keep track of the element .fringeWhitespace has
   to be added to; this is either dataElement or the generated paragraph
   or nothing, but we can't modify dataElement because it's used later
* In getDomFromData():
** When processing an opening, store the fringeWhitespace data in the
   generated DOM node
** When processing a closing, add the stored whitespace back in
* In the ve.dm.Document constructor, pass through .fringeWhitespace from
  the linear model data to the generated nodes

Tests:
* Change one existing test case to account for this change
* Add three new test cases for this behavior
* Add normalizedHtml field so I can test behavior with bare content

Change-Id: I0411544652dd72b923c831c495d69ee4322a2c14
2012-08-10 17:23:53 -07:00
Catrope d93b82347b Clean up a few commas and some indentation
The commas were resulting from converting this:
var foo = 3,
	bar = 5;

to this:

foo = 3,
bar = 5;

Change-Id: I0223b34a30d947c6a51f0601727b0c3850239e66
2012-08-10 16:49:14 -07:00
Timo Tijhof 23c5b0d02c Make use of new jshint options
* Restricting "camelcase":
  No changes, we were passing all of these already

* Explicitly unrestricting "forin" and "plusplus"
  These are off by default in node-jshint, but some distro of jshint
  and editors that use their own wrapper around jshint instead of
  node-jshint (Eclipse?) may have different defaults. Therefor
  setting them to false explicitly. This also serves as a reminder
  for the future so we'll always know we don't pass that, in case
  we would want to change that.

* Fix order ("quotemark" before "regexp")

* Restricting "unused"
  We're not passing all of this, which is why I've set it to false
  for now. But I did put it in .jshintrc as placeholder.
  I've fixed most of them, there's some left where there is no clean
  solution.

* While at it fix a few issues:
 - Unused variables ($target, $window)
 - Bad practices (using jQuery context for find instead of creation)
 - Redundant /*global */ comments
 - Parameters that are not used and don't have documentation either
 - Lines longer than 100 chars @ 4 spaces/tab

* Note:
 - ve.ce.Surface.prototype.onChange takes two arguments but never
   uses the former. And even the second one can be null/undefined.
   Aside from that, the .change() function emits
   another event for the transaction already. Looks like this
   should be refactored a bit, two more separated events probably
   or one that is actually used better.
 - Also cleaned up a lot of comments, some of which were missing,
   others were incorrect
 - Reworked the contentChange event so we are no longer using the
   word new as an object key; expanded a complex object into multiple
   arguments being passed through the event to make it easier to work
   with and document

Change-Id: I8490815a508c6c379d5f9a743bb4aefd14576aa6
2012-08-10 02:50:30 +02:00
Trevor Parscal d8ee3c2c29 After much research on error objects, native = good, custom = bad
Stack traces, line numbers, etc. All the approaches I've seen are bad hacks. This is the best way to go.

Change-Id: Ib12e9d2ecfe610bcc89d046005e35cc13efa3d99
2012-08-08 10:48:53 -07:00
Trevor Parscal b4de3ead08 Throw ve.Error instead of string literals
Throwing strings is bad because it doesn't include a lot of important
information that an error object does, such as a stack trace or where
the error was actually thrown from.

ve.Error inherits directly from Error. In the future we may create
more specific subclasses and/or do custom stuff.

Some interesting reading on the subject:
* http://www.devthought.com/2011/12/22/a-string-is-not-an-error/

Change-Id: Ib7c568a1dcb98abac44c6c146e84dde5315b2826
2012-08-08 06:19:00 +02:00
Timo Tijhof 9b49a7ce81 Clean up: Single quotes
* There were only 3 files with single quotes, fixed them all.

* Added option to .jshintrc (be sure to use the latest version of
  node-jshint since this is a fairly new addition to the library).

Change-Id: I8bf8895ce56bf86e3bed244279a9d32269e44763
2012-08-07 07:02:01 +02:00
Trevor Parscal 2eb0d4e51f Javascript, unlike PHP doesn't need double quotes for \n
This just cleans up a few places where single quotes should have been used instead of double quotes.

Change-Id: I6c53652e71ab96842ed5bb41fb1e0b8c923eb25d
2012-08-07 06:49:44 +02:00
Trevor Parscal 255ce870e2 Puttin' em white-spacers where they aught'a be
function() -> function ()
){ -> ) {

Change-Id: I20a85fcf79d7aec64f7f2559e84c0279550d4eea
2012-08-06 18:52:19 -07:00
Catrope 444961af2f Kill all but one of the Parsoid compat hacks, we don't need them any more
Also remove traces of Parsoid workarounds in tests. Tests are now
passing, yay :)

Change-Id: I8a59fc92c567c3595849e3e9377ce6eb6acde280
2012-08-06 13:56:40 -07:00
Trevor Parscal 13ccb68ae1 Cleanup - all jshint conditions are now met
Also:
* Removed a lot of dead code in Surface that was used in the now dead and gone sandbox.
* Changed from throwing an exception when calling getBalancedData on a range that produces no results from selectNodes to just returning []

Change-Id: Icf27094724eae5b90eec21308f9e26afe877e3ee
2012-08-03 18:56:04 -07:00
Timo Tijhof 88f6089952 Kranitor #1: On-boarding
'''Kranitor commits''' are commits by Krinkle with his janitor hat on.
Must never contain functional changes mixed with miscellaneous changes.

.gitignore:
 * Add .DS_Store to the ignore list so that browsing the directories
   on Mac OS X, will not add these files to the list of untracked
   files.
 * Fix missing newline at end of file

.jshintrc
 * raises -> throws
 * +module (QUnit.module)
 * remove 'Node' (as of node-jshint 1.7.2 this is now part of
   'browser:true', as it should be)

Authors:
 * Adding myself

MWExtension/VisualEditor.php
 * Fix default value of wgVisualEditorParsoidURL to not
   point to the experimental instance in WMF Labs.

Issues:
 * ve.ce.TextNode:
  - Fix TODO: Don't perform a useless clone of an already-jQuerified object.
  - Use .html() to set html content instead of encapsulating between
    two strings. This is slightly faster but more importantly safer,
    and prevents situations where the resulting jQuery collection
    actually contains 2 elements instead of 1, thus messing up
    what .contents() is iterating over.
 * ve.ce.Document.test.js
  - Fix: ReferenceError: assert is not defined
 * ve.dm.Document.test.js
  - Fix: ReferenceError: assert is not defined
 * ve.dm.Transaction.test.js
  - Fix: ReferenceError: assert is not defined
 * ve.dm.TransactionProcessor.test.js
  - Fix: ReferenceError: assert is not defined
 * ext.visualEditor.viewPageTarget
  - Missing dependency on 'mediawiki.Title'

Code conventions / Misc cleanup
 * Various JSHint warnings.
 * Whitespace
 * jQuery(): Use '<tag>' for element creation,
   use '<valid><xml/></valid>' for parsing
 * Use the default operator instead of ternary when the condition and
   first value are the same.
   x = foo ? foo : bar; -> x = foo || bar;
   Because contrary to some programming language (PHP...), in JS the
   default operator does not enforce a boolean result but returns the
   original value, hence it being called the 'default' operator, as
   opposed to the 'or' operator.
 * No need to call addClass() twice, it takes a space-separated list
   (jQuery splits by space and adds if needed)
 * Use .on( event[, selector], fn ) instead of the deprecated
   routers to it such as .bind(), .delegate() and .live().
   All these three are now built-in and fully compatible with .on()
 * Add 'XXX:' comments for suspicious code that I don't want to change
   as part of a clean up commit.
 * Remove unused variables (several var x = this; where x was not
   used anywhere, possibly from boilerplate copy/paste)
 * Follows-up Trevor's commit that converts test suites to the new
   QUnit format. Also removed the globals since we no longer use those
   any more.

Change-Id: I7e37c9bff812e371c7f65a6fd85d9e2af3e0a22f
2012-07-27 14:40:00 -07:00
Catrope 7f5dc887ea Do underscore/space conversion in internal link targets
Convert underscores in the href attribute to spaces in the linear model,
and back to underscores when going back to HTML. This ensures the link
targets displayed to and edited by the user look nice

Change-Id: I4855fce28ad8b724284c53881abc7b99b59b9079
2012-07-26 17:30:35 -07:00
Catrope 2b548a002f Get link title from href by stripping article path
This means we don't have to rely on data-rt.sHref. It also means that
we'll now be showing the canonical link target in the link inspector
rather than the link target as entered by the user, but that's fine.

Also change test to have href differ from sHref to show that we use
href.

Change-Id: Idabdbf2579663ef1efb47d6a73f39743c9f64f3b
2012-07-26 16:25:35 -07:00
Catrope 67e11ebbc3 Make VE work again with the link RDFa changes in Parsoid
This is ugly but makes things work again. I intend to clean this up once
we have a better attribute API

* Recognize mw:WikiLink, mw:SimpleWikiLink, mw:ExtLink,
 mw:NumberedExtLink and mw:UrlLink
* Support is incomplete because we can't get to the annotation text with
  the current API
* Preserve all unhandled attributes rather than special-casing data-mw
* Update remaining code using data-mw (sHref and stx extraction) to
  account for the data-mw -> data-rt rename
* Update tests accordingly

Change-Id: Ia13d3008a6d4cdc8828f9acda5aa797566bc597f
2012-07-26 16:23:03 -07:00
Catrope 1a573480f7 Moved text node test to the correct directory
Change-Id: I6c2146cd051e9e992e6a30a2d5f83d337d454387
2012-07-25 14:49:48 -07:00
Trevor Parscal 76bac7d152 Move test files
Change-Id: Id0a0bd5b4a91f702cad34e9f5e7f2121763abffd
2012-07-25 14:35:49 -07:00