Commit graph

212 commits

Author SHA1 Message Date
Trevor Parscal 8fc98868c9 Fixed inspector behavior
ve.ui.Inspector
* Removed disabled state and interfaces - this isn't needed
* Renamed prepareSelection to onInitialize
* Using event emitter to run onInitialize, onOpen and onClose methods
* Left removal up to the child class to handle in the onClose method
* Replaced calls on context to close inspector to calling close directly
* Renamed prepareSelection stub to onInitialize
* Emitting initialize event from within the open method
* Added recursion guarding to close method
* Changed the close method's argument to be remove instead of accept - the more common case is to save changes, and the only time you wouldn't save changes is if you were to remove the annotation
* Moved focus restore to close method

ve.ui.Context
* Moved the majority of the code in openInspector and closeInspector to event handlers for onInspectorOpen and onInspectorClose
* Updated calls to closeInspector re: accept->remove argument change

ve.ui.LinkInspector
* Renamed prepareSelection to onInitialize and rewrote logic and documentation
* Removed unused onLocationInputChange method
* Moved restore focus (now it's in the inspector base class)

ve.dm.SurfaceFragment
* Added word mode for expandRange

ve.dm.Surface
* Added locking/unlocking while processing transactions - this was not an issue before because this was effectively being done manually throughout ce (which needs to be cleaned up) but once we started using the content action to insert content dm and ce started playing off each other and inserting in a loop - we already do this for undo/redo so it makes sense to do it here as well

ve.InspectorAction
* Updated arguments re: close method's accept->remove argument change

Change-Id: I38995d4101fda71bfb2e6fe516603507ce820937
2012-11-21 12:01:14 -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
Trevor Parscal 96fcbbd695 Cleanup var statements
Put non-assignment vars first

Change-Id: Id868976bc4d7c21032aef5020fe5fd0eb5df7a14
2012-11-19 17:05:34 -08:00
Catrope 22a01bfc4f Merge "The great inspector and context rewrite of 2012" 2012-11-19 23:23:48 +00:00
Trevor Parscal d2476a26d2 The great inspector and context rewrite of 2012
ve.AnnotationAction
* Added filter to the clearAll method to allow clearing all matching annotations only

ve.dm.Document
* Some variable renaming for consistency

ve.dm.SurfaceFragment
* Added truncateRange method
* Added annotation scope to expandRange method
* Added support for passing an annotation object into annotateContent method
* Switched to using name instead of type in annotateContent method to match the ve.dm.Annotation class
* Fixed logic in annotation mode of expandSelection so that expansion only takes place if the annotation is found

ve.ui.LinkInspector
* Moved most of the functionality elsewhere
* General reorganization
* Changed setOverlayPosition to accept 2 arguments instead of an object with 2 properties and renamed it to positionOverlayBelow
* Check for annotation object before extracting target information from it
* Initialize default target as empty string to avoid undefined being cast to a string and the default target becoming 'undefined'

icons.ai, inspector.png, inspector.svg
* Added generic inspector icon which will be used when a custom icon is not specified in future inspector subclasses

ve.ui.Inspector.Icons
* Added inspector icon
* Renamed clear icon to remove to match it's actual image

ve.ui.Context
* Greatly simplified the interface, reducing the number of methods by inlining a few things and combining others
* Now always listening to resize events on the window rather than only while document is focused
* Not listening to scroll events anymore, they used to affect the top/bottom positioning of the menu which we don't do anymore
* Lots of cleanup and reorganization
* No need to fallback to empty array since getInspectorsForAnnotations does so already
* Only consider fully-covered annotations for inspectors

ve.ui.Frame
* Simplified the constructor by introducing the createFrame method
* General cleanup
* Typo fixes

ve.ui.Inspector
* Generalized lots of functionality previously located in the link inspector class which will be useful to all inspectors (such as title, clear button, saving changes, etc.)
* Added setDisabled and isDisabled methods to manage CSS changes and avoid needing to check the CSS to determine the state of the inspector (storing state in the view is evil)
* Added getMatchingAnnotations method for convenience
* Added prepareSelection stub
* Lots of cleanup and documentation
* Type pattern is now defined in base class
* Added stubs for onOpen and onClose with documentation so that subclass authors know what these methods do
* Removed checks for onOpen or onClose methods since they are now noop stubs and are always there
* Added stub and removed checks for onRemove
* Made esc key close and accept - the illusion is supposed to be that the link changes are applied instantly, even though they are only updated when you close, so all closing except for when removing should apply changes - i.e. esc is now equal to back rather than being a special function that doesn't have an associated affordance
* Only consider fully-covered annotations when getting matching annotations

ve.ui.InspectorFactory
* Depending on type pattern now since it's always there
* Added getInspectorsForAnnotations method
* Return empty array if annotation set is empty

VisualEditor, VisualEditor.i18n
* Added default inspector message

Change-Id: I1cc008445bcbc8cba6754ca4b6ac0397575980d5
2012-11-19 15:21:27 -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 3acc6cb8f4 Disable change marking by default
It's causing problems with Parsoid in production

Change-Id: Id47493baafe1ec7f7c0e2bbdb2ea60a82913dfaf
2012-11-14 11:58:32 -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
Rob Moen 5d86bc4379 Base for inspectors & context menu to operate on 0 length selection.
* Allow inspector to open with 0 length selection.
* Allow context menu to open with 0 length selection.
* Fixed bug in doc.getAnnotationsFromRange on zero length selection:
  Method now returns annotations from start vs empty annotation set.

Change-Id: I3937c5c2824c7396d0c3ee11c13ffecdbed6052a
2012-11-02 13:08:32 -07:00
Trevor Parscal 735ed96f5f Add ve.Action, ve.ActionFactory, subclasses
Moved implementation of all the tools into a reusable action
system. To execute an action just call

surface.execute( actionName, method, param1, param2, ... );

This helps keep tools simple, and opens the door to key commands
reusing the same code.

Change-Id: Ie786fa3d38d1ea17d39b5dfb8eeeb5f2256267ce
2012-10-26 14:44:17 -07:00
Trevor Parscal ed7273da25 Add ve.dm.Surface.has{Past,Future}State, add docs
Will be used by the history tools in a future commit, providing
a reasonable interface to this information rather than the tool
reaching into private members.

Change-Id: I0472349968e9b48ec17eb47b6845ec9ccf3811e2
2012-10-26 14:43:09 -07:00
Trevor Parscal fcbe1d7b9c Add ve.dm.Node.hasAttributes()
Will be used in future commits to reduce duplication.

Change-Id: Ibde934d5a55b980010e84b6242f8d968e88cea54
2012-10-26 14:42:40 -07:00
Trevor Parscal 79944333a4 Add ve.dm.Node.hasMatchingAncestor()
Will be used in future commits to avoid traversing up the tree and
comparing type and attributes.

Change-Id: I993f4cbe20018406dbb4a3c9d1ce2f8766f8e1ba
2012-10-26 14:41:49 -07:00
Trevor Parscal 7d741253f3 Add ve.dm.SurfaceFragment.getAnnotations()
Change-Id: Ifb731e8fdd0f111f5da4ac0d882adaad3f48fdce
2012-10-26 11:03:51 -07:00
Catrope bc8b042e5f Merge "Fix annotation object creation in SurfaceFragment" 2012-10-26 17:53:10 +00:00
Krinkle b6b899c396 Merge "Documentation fixes" 2012-10-25 21:19:05 +00: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 551a30bc64 Merge "Actually use nodeOuterRange, selectNodes() has provided this forever" 2012-10-25 18:00:15 +00:00
Trevor Parscal fe5c5b78f9 Merge "Reorganize text-only replacement detection" 2012-10-25 17:56:45 +00: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 f1dbdac3d8 Actually use nodeOuterRange, selectNodes() has provided this forever
Change-Id: Ie1d3cbb6e475248927d10f411cf383f701a3ad71
2012-10-24 17:48:48 -07:00
Catrope 5d5335b498 Reorganize text-only replacement detection
* Lift node assignment out of the if/else
* Flip the condition so we detect text-only replacements rather than
  non-text-only replacements
* Additionally assert that there is exactly one selected node, and that
  it is a text node

Change-Id: Iaaddf532f06709e860ac44457470e6d8bfcb6dd9
2012-10-24 17:48:20 -07: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 8c3802308e Fix annotation object creation in SurfaceFragment
Also add a comment about how the way data is passed to a new annotation
needs to be improved

Change-Id: I693e4f5632b4eae0d2ee3468ee979454314f6364
2012-10-24 16:47:04 -07:00
Trevor Parscal f17fc65c41 Documentation fixes
Change-Id: I5020623ba59cd9d1794b7d1f97d3adf77b833d6e
2012-10-24 16:47:04 -07: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
Trevor Parscal 8ea206c743 Removed htmlTagName and htmlAttributes from hash
This was only causing data bloat and also errors because htmlTagName and htmlAttributes are only set if the annotation was constructed from an HTML element

Change-Id: I3d36bca6cd0194e1a4456bb51156117f70b96b13
2012-10-19 16:01:28 -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 5cd4222d9c Whitespace and comment cleanup for dm annotation classes
* Added comments to classes and methods
* Quieted a jshint warning
* Broke some long lines
* Replaced instances of "var\t" with "var "

Change-Id: I1d617ed9e5180f1a3dff42078fb5debb5d718407
2012-10-17 12:35:28 -07:00
Trevor Parscal 1c5ac9d502 Merge "Fix exception when deleting all text (Ctrl+A Backspace)" 2012-10-17 19:33:48 +00:00
Catrope 0054cd2a57 Fix exception when deleting all text (Ctrl+A Backspace)
Exception was caused by passing -1 to getAnnotationsFromOffset(). So
check for -1 before passing it in; getNearestContentOffset() can
legitimately return -1 if there are no content offsets in the document,
which occurs when the document is empty.

I was originally going to change getNearestContentOffset(start - 1, -1)
to getRelativeContentOffset(start, -1), but Inez correctly pointed out
that that would have unwanted results when near an inline node.

Change-Id: Ife4b497b1c5fd04d411bb25cea99e6ea2abf146f
2012-10-15 15:03:08 -07:00
Catrope 91964a48d0 Fix JS error in DocumentSynchronizer when adding to an empty document
This was reproducible by blanking the entire document (Ctrl+A Delete),
then undoing that (Ctrl+Z). AFAIK that's the only way to trigger an
insertion on a document that is completely empty.

Change-Id: I22252d5972a413dff614880a90c4c6b22e79672d
2012-10-15 15:01:39 -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 735ee449e3 New annotation API: ve.dm.Converter integration
The annotation-related code in the converter is greatly simplified
because the API itself takes care of almost everything already.

Change-Id: Ib48f52bad6b650a05dc4e7ef82db4158c19b3cf5
2012-10-12 15:07:28 -07:00
Catrope 613dd14332 New annotation API: convert existing annotations
This changes ve.dm.LinkAnnotation to be a generic annotation for <a>
tags, and adds ve.dm.MWInternalLinkAnnotation and
ve.dm.MWExternalLinkAnnotation as MW-specific subclasses. This nicely
splits out the MW-specific parts in LinkAnnotation, and ideally we'd
also move these files somewhere else to reflect their MW-specificity,
but I haven't gotten to that yet.

Similarly, ve.dm.TextStyleAnnotation is now a generic base class for
simple tag-only-no-metadata annotations, and it has 11 subclasses, one
for each tag we support. This is quite a bit more verbose than the
previous code, but I think it's cleaner and more flexible. I considered
writing a function that would generate a TextStyleAnnotation subclass,
then calling that 11 times, but that's not possible if we want to keep
named functions for the constructors.

Change-Id: Ifba10153eef40280e44025dd72d4e9d9f33b0632
2012-10-12 15:07:25 -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
Timo Tijhof a15b2f77f2 Fix constructor names; remove redundant hasOwnProperty.
Add some missing constructor names and rename the ones with a
lowercase 'v'.

I previously changed Object.create and others to using hasOwn,
but that turned out to be useless. The thought at the time was
to only use the native one if it really is a native one (and not
a polyfill from another script), however in then hasOwn is only
relevant on prototypes and when negated. For static members it
would be an own-property either way.

Follows-up:
* Id6783fcfc35a896db088ff424ff9faaabcaff716 (metanode)
* Iab763954fb8cf375900d7a9a92dec1c755d5407e (object-management)

Change-Id: Ia6ef597e5e5453277472dfc23f25d2878b68b7f6
2012-10-08 06:15:20 +02:00
Catrope f434da5140 Merge "Changed method name to setAutoSelect to avoid collision" 2012-10-05 21:25:51 +00:00
Trevor Parscal da2d89ee3a Changed method name to setAutoSelect to avoid collision
Plus, this is more consistent with the rest of the method names

Change-Id: I713f1bfc28d22cc259ce5dba69ea053817fd43f8
2012-10-05 11:07:10 -07:00