To help the selective serialiser we can return the original
HTML for generated content if it is unmodified.
As the output of toDomElements now depends on changes
to the dataElement we now have a 'modify' function in
the some test cases.
We also now have 'storeItems' to assert that the index-value
store is correctly populated and for loading values back
into the store for toDomElements tests.
Also make 'mw' an attribute and remove 'about' property.
Bug: 47394
Change-Id: I2bbb5d2d6a90c4eb87fa129671112c92a9b931e7
Also add detection for whether the browser is actually broken (most are,
but some, like Opera, aren't), treat <textarea> and <listing> in addition
to <pre>, and fix a bug where the function would crash if the <pre> was
empty (because .firstChild was undefined/null).
Change-Id: I541b57e9fd5c9c42d19d0a59f6e29fb43d35c9b6
addSet:
* Instead of indexing items in the store, just union the indexStore arrays
removeSet/removeNotInSet:
* difference or intersect the indexStore arrays
filter:
* push indices into the result set instead of values
simpleArrayUnion/Intersect/Difference have been created as utilities
in ve. They are prefixed 'simple' because they use object keys to
do fast in-array comparisons. This means they are limited to string
values or values which will compare as strings (e.g. numbers).
Change-Id: I079cbdfece4f6d80ec0afd61959913f13217fcb3
By removing the transaction listeners from surface fragments we
no longer have to make sure they are always manually destroyed.
In order to retain the functionality of having fragments update
with transactions elsewhere we keep a pointer to a place in the
new complete history stack in the surface. The complete history
stack records all transactions, even undone ones.
Whenever getRange is called we replay all transactions in the
complete history (in the correct order) since the fragment was
last updated.
Also in this commit:
* Updated Format/IndentationAction to test undo(). This increases
coverage of surface fragment behaviour.
* .range is always accessed by .getRange now, although as an
optimisation we can use the noCopy mode when we a sure the
returned range will not be modified.
* Added undo test to .update (previously .onTransact)
Bug: 47343
Change-Id: I9e9818da1baa8319a3002f6d74fd1aad6732a8f5
This was broken, especially in wrappers.
Changed the wrapping algorithm so that meta items are placed outside
wrappers if possible. On the left-hand side, this is already the case:
we don't open wrappers for meta items. On the right-hand side, this is
accomplished by buffering the meta items and only inserting them when
we encounter either real text (not whitespace) or the end of the wrapper.
If we're interrupted by real text, we insert the meta items with the
unmodified whitespace. If we're interrupted by the end of the wrapper,
we insert the meta items outside of the wrapper with whitespace stripped.
Internally, this is done by stripping the whitespace into the whitespace[0]
of the meta item to its right. Then when we output the meta items, we
either decide to 'restore' the whitespace, or to 'fixup' by also setting
whitespace[3] on the element before the whitespace.
Change-Id: Ibeea2a9906c4aae9fe6d284613edd6ec853ca5e7
So. It turns out that the design of SurfaceFragment is a little -
shall we say - wonky.
One of the best things about ve.dm.SurfaceFragment is its magical
ability to retain the intention of its range, even as transactions
are being processed. This ability is granted by each fragment
listening to the surface's change event, and responding by using
translateRange for each transaction that gets processed. Surface
fragments also have these clever methods that allow you to get a
fragment based on another, which makes adjusting the range easy to do
inline without having to manually store multiple fragments or
modifying the original.
This sounded good, and we seemed to all be convinced it was well
designed. But if you add a console.log( 'hello' ); to the first line
of ve.dm.SurfaceFragment.prototype.onTransact, and then start using
the bold tool on various selections of text, you will find that there
may indeed be a flaw. What you will probably realize is that the
number of times that particular line of code is being called is
disturbingly large, and increases each time you do just about anything
in the editor. What's going on? How did we get here? Read on…
It turns out that fragments are immortal. We create them, they listen
to the surface's transact event, we are done with them, but the
surface keeps on emitting events to the now long forgotten about
fragments. They continue to build up over time, never go out of scope,
and bloat the hell out of our program.
The same ended up being true of toolbars - and each time the context
menu fired up a new one the old one was left in limbo, still
responding to events, still taking up memory, but not being visible to
the user.
All of this immortality was causing strange and difficult to track
down problems. This patch fixes this by introducing a destroy method.
This method unbinds events, allowing the object to finally fall out of
scope and die - and more importantly stop receiving notifications of
changes.
This is a hack, but Ed will no doubt get this situation sorted out
properly by making fragments lazy-evaluate their selections by only
storing an identifier of the most recent transaction they were based
on, see bug 47343.
Change-Id: I18bb986001a44732a7871b9d79dc3015eedfb168
Before, it took an array of objects and translated those to indexes
using the store. Literally every caller outside of the test suite got
an array of indexes from the linear model, translated those to objects,
then passed them into the AnnotationSet constructor which translated
them right back to indexes.
The previous behavior was kind of ridiculous on its face, but the
reason we found it is because Inez was investigating the performance
degradation when bolding a line and found that half of it was due
to the hundreds of ve.getHash() calls caused by this behavior.
Change-Id: I38df8ae9f6392849dacf477ea2f804283c964417
Subbu said that cloning of attributes like data-parsoid or typeof would
cause problems for Parsoid.
Also remove the attributes object if it becomes empty, and do the same
for the internal object.
Bug: 47297
Change-Id: I428becf95c70d0ed8af5b0c408e3966dc47fd8c3
Context-sensitive nodes are HTML elements like <caption> that can only
appear in certain contexts (<table> in this case). This means that
serializing them by throwing them in a <div> and calling .innerHTML
fails, because the browser knows a <caption> can't be in a <div> and
unwraps it. jQuery's .html() function is clever though and knows to wrap
<caption> in a <table> (and has similar rules for other elements).
So use jQuery's .html() rather than manual .innerHTML stuff.
Change-Id: Id7d3eff968b3a2ba345680772f7cc32e3dcdb529
In HTML like <td>Foo <b>Bar</b></td>, the space would be stripped and
registered as trailing whitespace in the <td>, so it wouldn't be visible
in the editor and would be inserted after the </b> on the way out.
Thanks to Subbu for reporting this. This case was kind of ridiculous and
we're lucky the JRuby article contained it. To trigger the bug, you had
to have:
* a table cell
* containing unwrapped content
* consisting of
** some text
** whitespace
** open annotation (bold in my example, link in Subbu's case)
** text
** close annotation
** and nothing else
Change-Id: I2b83f02764b311a32a50956d4c8930a9394e91a4
The first run of getDomFromData() would preserve whitespace just fine,
but it blanked out the .veInternal.whitespace[1] element in certain
cases, contaminating the linear model and making the whitespace data
inconsistent. Subsequent runs of getDomFromData() would then refuse to
serialize that whitespace because the information about it was
inconsistent.
In getDomFromData(), we sometimes unset .veInternal.whitespace[1] (i.e.
set it to undefined) to prevent double processing. Because we're
potentially going to modify .veInternal, don't assign it by reference,
but copy the object.
Added tests asserting that the linear model is unchanged after calling
getDomFromData(), because that function should never modify
linear model data. This test failed in 4 cases (all whitespace-related)
before I added the copyObject() call.
Bug: 43543
Change-Id: Ic4c93510518163894201a693ab50331413715967
ve.dm.Transaction
* Replace operations are now built directly from the
linear model and automatically determine what metadata
replace information they need to include:
** retainMetadata
** replaceMetadata
** insertMetadata
ve.dm.Document
* Metadata array created empty and padded out after data parsing
as we are no longer using Document.spliceData to build it (a new
test checks for correct metadata length)
* spliceData replaced with getMetadataReplace, which instead returns
transactional steps of spliceData (retain, replace, insert)
ve.dm.MetaLinearData
* Add function for merging metadata items together. Only used
once in the code (Document.getMetadataReplace) but useful
for generating test data.
ve.dm.MetaList
* Replace operations with metadata need to calculate new offset
and indices directly, but can't be applied immediately lest they
put a metaItem out of place and affect findItem.
ve.dm.MetaItem
* Add methods to support queued moves as required by MetaList
Test files
* Updated to match new pushReplace API
* Remove any instances of Document.spliceData
* Extra check on sparse metadata array length
* Rewrite spliceData tests as getMetadataReplace tests
* Count expected cases in Transaction(Processor) tests
Bug: 46954
Change-Id: I4edad1c2dd37c723bff2792bab7d694ef17a86dc
* Create MWTemplateBlockNode & MWTemplateInlineNode (in ce and dm)
* Move Alien's 'isInline' code to ve.dm.Node.static.isHybridInline
* Move definition of ce.AlienBlock/InlineNode inside ce.Aline.js file
to match dm.AlienBlock/InlineNode and MWTemplate
* Duplicate AlienBlock/Inline styles for templates
* Create test case for inline templates
* Count test cases in ve.dm.Converter.test.js automatically
Change-Id: Id9bc7f049ea974dd5e7f8b7a66080939e0948bbd
This node stores the rendered in the index-value store, hashed on
a custom hash of the dm (type + mw) which makes it unique it its
parameters.
Bug: 46571
Change-Id: I0ab4c9f7bca207121d5b42e83c821771b6139da8
ve.ce.ImageNode.js
* Moved in generic stuff from MWImageNode
* Added drag end handler (empty, will be used soon)
ve.ce.MWImageNode.js
* Changed to inherit ImageNode
* Moved generic stuff out
ve.dm.ImageNode.js
* Added attribute extraction/preservation for src, width and height
ve.dm.MWImageNode.js
* Changed to inherit ImageNode
* Re-using ImageNode's attribute handling to extract/preserve attributes on both the image and wrapper level
Change-Id: Ied4e1ece24e6804220eac35330790f7084df55de
It's been passed in for a while, but nothing ever used it. As we know
some browsers don't like it when we create elements in the wrong
document, and this ensures we always use the correct document for
createElement().
Change-Id: Ia3d2fabe0516956105ad2b5625ed2f76c015c26e
They worked fine with just one group, but once Rob added a second group
everything came falling down.
* Index the return value of findItem() into the correct array
(list.items for all, list.groups[groupname] for groups)
* Expect null if there is something at the coordinates but it's in the
wrong group
* Keep track of the next index and expect that for forInsertion when
findItem() returned null, as well as at the end of each offset
Change-Id: I76438f583cea5694ce04fa2f4e7e88f8f8f236d1
This changes the annotation API to be the same as the node API, sans
a few boolean flags that don't apply. The APIs were different, but
there was really no good reason why, so this makes things simpler for
API users. It also means we'll be able to factor a bunch of things out
because they're now duplicated between nodes, meta items and annotations.
Linear model annotations are now objects with 'type' and 'attributes'
properties (rather than 'name' and 'data'), for consistency with elements.
They now also contain html/0/* attributes for HTML attribute preservation,
which obsoletes the htmlTagName and htmlAttributes properties.
dm.Annotation subclasses take a reference to such an object and implement
conversion using .static.toDataElement and .static.toDomElements just
like nodes do. The custom .getHash() functions are no longer necessary
because of the way HTML attribute preservation was reimplemented.
CE rendering has been moved out of dm.Annotation (it never made sense to
have CE rendering functions in DM classes, this was bothering me) and into
separate ce.Annotation subclasses. These are very similar to CE nodes in
that they have a this.$ generated based on something in the DM; the main
difference is that nodes listen to events and update themselves, whereas
annotations are static and are simply destroyed and rebuilt when they
change. This change also adds whitelisted HTML attribute rendering for
annotations, as well as class="ve-ce-FooAnnotation" attributes.
Now that annotation classes produce real DOM nodes rather than weird
objects describing HTML tags, we can't generate HTML as a string in
ce.ContentBranchNode anymore. getRenderedContents() has been rewritten
to be much more similar to the way the converter renders annotations;
in fact, significant parts of it were copied from the converter, so that
should be factored out in the future. This change actually fixes an
annotation rendering discrepancy between ce.ContentBranchNode and
dm.Converter; see the diff of ve.ce.ContentBranchNode.test.js.
ve.ce.MWEntityNode.js:
* Remove stray property
ve.dm.MWExternalLinkAnnotation.js:
* Store 'rel' attribute
ve.dm.TextStyleAnnotation.js:
* Put all the conversion logic in the abstract base class
ve.dm.Converter.js:
* Also feed annotations through getDomElementsFromDataElement() and
createDataElement()
ve.dm.Node.js:
* Fix undocumented property
ve.ce.ContentBranchNode.test.js:
* Add descriptive messages for each test case
* Compare DOM trees, not HTML strings
* Compare without all the class="ve-ce-WhateverAnnotation" clutter
ve.ui.LinkInspector.js:
* Replace direct .getHash() calls (evil!) with ve.getHash()
Bug: 46464
Bug: 44808
Change-Id: I31991488579b8cce6d98ed8b29b486ba5ec38cdc
* In matchTypeRegExps(), skip string types
** Didn't break because we currently have mixes of strings and regexes
* Combine types from rel, typeof and property rather than picking one
** Added test case in ve.dm.example that resembles actual Parsoid output
* If the element has extension-specific types, not only restrict type
matching to extension-specific types, but also require that *all* types
present on the element be matched
Change-Id: Iacf3851a0ca9081d2c813b42435484a47cec6230
Created an IndexValueStore class which can store any object and return
an integer index to its hash map.
Linear data is now stored in ve.dm.LinearData instances. Two subclasses
for element and meta data contain methods specific to those data types
(ElementLinearData and MetaLinearData).
The static methods in ve.dm.Document that inspected data at a given
offset are now instance methods of ve.dm.ElementLinearData.
AnnotationSets (which are no longer OrderedHashSets) have been moved
to /dm and also have to be instantiated with a pointer the store.
Bug: 46320
Change-Id: I249a5d48726093d1cb3e36351893f4bff85f52e2
Using element.height was returning 0 if the attribute was empty
when in fact what we mean to store is null (i.e. auto height).
This takes care of the writing of attributes in CE as jQuery
ignores an attribute-set command if the value is null.
Also in this commit I've implemented a basic toDomElements
that outputs the original HTML (code copied from AlienNode).
This stops the code from throwing an exception but will
eventually need to be rewritten to rebuild the HTML from
the attributes stored in the DM.
Bug: 56336
Change-Id: I297a1d0a07e9ebf9d0110fb1cdf266f8415f25b7
Initially just with a Wordbreak module to implement Unicode standard
on 'Default Word Boundaries'. Due to it's standaloneability this has
been written as a separate library. Non-BMP characters are currently
not supported.
Bug: 44085
Change-Id: Ieafa070076f4c36855684f6bc179667e28af2c25
Add .insertMeta() and .removeMeta() methods to insert and remove
metadata through the meta list. For convenience, there is also a
.remove() method in MetaItem that wraps around removeMeta().
Also rename insertItem() and removeItem() to addInsertedItem() and
deleteRemovedItem() to avoid confusion, and make the MetaList
constructor take a dm.Surface rather than a document so we can call
change().
Change-Id: I4b62a3109404cfd56f5de68938e1db908b03e678
Previously these were static functions in TransactionProcessor
which instantiated a TP called .process() on it. These are now
methods of ve.dm.Document.
Also moved the emission of the 'transact' event on the document from
TransactionProcessor to Document itself, and moved the tests asserting
double application is protected against from TP to Document (because
the corresponding code moved as well).
Change-Id: I7c9f22a14accaf0ba1f70d5aa4f0573bb7e677d0
This is just a prototype - if it will be as useful as I expect
then it will get refactored (and probably get a better name +
location).
Change-Id: Ice1a2bd7d498d9d8438c35239216f01bd3db1826
A MetaList is a collection of MetaItems representing all of the
metadata in a ve.dm.Document, and it updates itself live as the
underlying document changes.
Currently this interface is read-only, I'll add mutators next.
Change-Id: If7bfc9563af37e22dcdca9a682d6decc2f6f1872
Rather than meta-things being special kinds of nodes, they are now a
separate class of things (MetaItems) along with Nodes and Annotations.
* Created a generic ve.dm.MetaItem that meta items inherit from.
There will be actual instances of this class as well in the upcoming
meta group code.
* Renamed MetaNode to AlienMetaItem, MWMetaNode to MWMetaItem,
'metaBlock'/'metaInline' to 'alienMeta'
* Created a MetaItemFactory, handle meta items in the ModelRegistry
* Kill ve.dm.Node.static.isMeta, now obsolete
ve.dm.Converter:
* Pass in the MetaItemFactory
* Look up data element types in the ModelRegistry rather than the
NodeFactory, because they can be either nodes or meta items
* Document createDataElement() and make explicit that modelClass can be
either a node or a meta item
* Handle meta items in getDataFromDom()
* In getDomFromData(), check the MetaItemFactory as well as the NodeFactory
Change-Id: I893709c6f3aa00f85c1b905b70f9f4e597bdeada
Replacing [a-zA-Z] with a long unicode expression which encompasses
all the characters in the unicode 'letters' category. Similarly replacing
[0-9] with an expression for 'numbers'.
Bug: 44085
Change-Id: Idd403339caa24769ce08133dda06ab6d4b9d694e
Also in this commit is a minor fix to the regular expression so it
behaves as documented (the hyphen needed escaping).
Bug: 44085
Change-Id: Idc315e2dce79be8f028b5681c60f74e175b9d869
isolateAndUnwrap now unwraps to a level determined by a target type
i.e. the type you are going to convert to.
Also in this commit wrap/unwrap/rewrap have been refactored to use
getLengthDifference. unwrap now takes an inner/outer unwrap depth.
Change-Id: I3c6249de43232a9ef64f498a0aaf66b1c44973f2
Heading and Preformatted nodes have rules that should only
exist under a document node in MediaWiki.
Two new node types have been created as has a new DropdownTool which
uses these. The MW init options have been changed to use the new
DropdownTool.
Bug: 45295
Change-Id: I3f47e1ae1f5c1415bde58a75385e4bf5f4b8fffc
Add a static.name property to ce nodes and make sure both ce
and dm nodes always use the static.name property in constructors
and registration calls.
The result of this is that any given node type should now only
appear once in the code as a string.
Bug: 45701
Change-Id: Ibf31de16ab28ad58209c1443cd74f93dda278998
rewrapAllNodes is effectively the same as unwrap then wrap
except it operates as one transaction as so avoids a potentially
invalid intermediate state.
Added some more comments for unwrapAllNodes and renamed a varaible
for consistency.
Fixed a typo in Transaction.newFromWrap comment.
Bug: 45242
Change-Id: Ie752a788d087055d97c7c6f75f59c6a2680d26c7
Previously, we would translate to the right of an insertion, but for
wrapping transactions that means we end up with something like
<ul><li>|<p>...</p></li></ul>|, which doesn't make any sense. This
change changes this to <ul><li>|<p>...</p>|</li></ul>.
* Add parameter to translateOffset() that toggles the behavior for
the offset before an insertion
* In translateRange(), translate start and end differently
** In some cases this can map them across each other, fix that
Change-Id: Ia2197b08d9f6763be3f2db5a59546ddc3f74a281
We already combine consecutive operations of other types, but didn't do
that for remove. We have to do this, though, because translateOffset()
gets confused otherwise.
Change-Id: I4285a3efd9f2297398f57e2f24adb26adafdf465
This changes the node API to work with multiple elements, so we can
support about groups. Instead of passing in and returning single DOM
elements, we use arrays of DOM elements.
ve.dm.Converter:
* Pass modelRegistry into the constructor
* Remove onNodeRegister handler and its data
* Remove getDataElementFromDomElement() and
getDataAnnotationFromDomElement(). Most logic moved into
getDataFromDom(), some into createDataElement()
* Remove createAlien(), replaced with
createDataElement( ve.dm.AlienNode, ... )
* Replace doAboutGrouping() (which wrapped about groups) with
getAboutGroup() (which returns an array of the nodes in the group)
* Put in a hack so <meta>/<link> elements with an mw: property aren't
alienated
* Remove about group wrapping behavior in favor of just outputting
multiple nodes
ve.dm.AlienNode.js:
* For multi-element aliens, only choose inline if all elements are
inline, not just the first one
ve.dm.example.js:
* Add html/0 stuff for meta nodes
* Fix test case to reflect new alien behavior
Change-Id: I40dcc27430f778bc00a44b91b7d824bfb2718be6
wrapAllNodes was calulating the new selection incorrectly. This has
been fixed and a test added.
unwrapAllNodes takes a depth as its argument and unwraps that many
elements from inside the selection.
Tests for wrap/unwrap apply also now check that applying a wrap
and then its inverse as an unwrap result in the document reverting
to its original state.
Change-Id: I7dcacdfb5894be59ffad69b369d7b32933a25b61
In our test case, offset 12 was mapped to 16, but that should be 11.
The problem here was that the offset right before the removal was
mapped to right after the removal, but that's only valid if we're
dealing with a removal, not when we're dealing with a replacement
where we're both removing and inserting data.
Change-Id: Ibf3c1463c0de009578cd50736f19bae82669ced8
This method will take a selection of siblings and ensure they
are the only chlidren in their first parent which can be placed anywhere
(for example the first parent of a tableCell which can be placed anywhere
is a table, and for a listItem is a list).
The method ensures no redundant empty tags are created, so if
the selection encompasses all siblings then no action is taken.
Also in this commit are two test cases run against ve.dm.example.isolationData.
Change-Id: I783bd5ecd9d43d61f9b2685985409b4d746cbe94
The Parsoid output will also be expected to be a full HTML document. For
backwards compatibility, we allow for the Parsoid output to be a
document fragment as well. We don't send a full document back yet, also
for b/c -- we'll change this later once Parsoid has been updated in
production.
ve.dm.Converter.js:
* Make getDataFromDom() accept a document rather than a node
** Split off the recursion (which does use nodes) into its own function
** For now we just convert the <body>. In the future, we'll want to do
things with the <head> as well
* Pass the document around so we can use it when creating elements
* Make getDomFromData() return a document rather than a <div>
ve.init.mw.Target.js:
* Store a document (this.doc) rather than a DOM node (this.dom)
* Pass around documents rather than DOM nodes
* Detect whether the Parsoid output is an HTML document or a fragment
using a hacky regex
* When submitting to Parsoid, submit the innerHTML of the <body>
ve.init.mw.ViewPageTarget.js:
* s/dom/doc/
* Store body.innerHTML in this.originalHtml
ve.Surface.js:
* s/dom/doc/
demos/ve/index.php:
* Don't wrap HTML in <div>
* Pass HTML document rather than DOM node to ve.Surface
ve.dm.Converter.test.js:
* Construct a document from the test HTML, rather than a <div>
ve.dm.example.js:
* Wrap the HTML in the converter test cases in <body> tags to prevent
misinterpretation (HTML fragments starting with comments, <meta>,
<link> and whitespace are problematic)
Change-Id: I82fdad0a099febc5e658486cbf8becfcdbc85a2d
Have created builders for insertion, removal, and single element replacement.
In adding Document.getMetadata which is nearly identical to Document.getData,
the two functions have been refactored to use a common static method
getDataSlice, with this.data/this.metadata as an argument.
Document.spliceMetadata has been added. It is essentially spliceData with
the data/metadata synchronisation issue.
Metadata cursor position is now tracked in the TransactionProcessor. Cursor
advancement has been moved to a function so the metadata cursor can be reset
every time the data cursor is moved.
There were unhit bugs in the TransactionProcessor run test section, where
the data being loaded into the test documents wasn't always being deep-copied,
and the assert was looking at getData instead of getFulldata (which wouldn't
be able to test metadata changes).
Change-Id: Ieb20ab3e7827bc7ff04148f147da6728eb1eb666
This means that <p data-foo="bar"> will now be converted to a paragraph
with attributes {"html/0/data-foo":"bar"} rather than {"html/foo":"bar"}
This paves the way for multi-element node (about group) handling in the
node API: nodes representing multiple DOM elements will have html/i/attr
to represent an attribute of the i'th DOM element.
Change-Id: Iea52bdccd721942ca708c8f9f47e934524809845
When encountering an inline node (i.e. content node that's not a text
node) within a branch node that's not a content branch node, the
converter should start a wrapper. But it doesn't do this, it only opens
wrappers for text nodes and annotations.
Fixed this in the converter, added a test for it, and fixed an existing
test that asserted the broken behavior.
Change-Id: I6e143e21e68b68f0d85b8772e24a2d3a5d465410
* Introduce context object as specified for
ve.dm.Node.static.toDataElement()
* Remove wrapping variable in favor of context.wrapping
* Remove wrappingIsOurs in favor of context.canCloseWrapper
* Introduce originallyExpectingContent and use it to repopulate
context.expectingContent after closing a wrapper
* Replace most uses of branchHasContent with context.expectingContent
** Except for two cases where we need originallyExpectingContent
These changes fix a case where a metaBlock was generated in an inline
position. Updated the tests to reflect this.
Change-Id: I6baf6053f8a3a0b7d91487f812b9235a7b2b3db1
Continues where Ibb682332a6084e357104183641a104e3ae1e253f left off, adding tests and removing inconsistencies between the behavior of the document constructor, which was adding empty text nodes to empty paragraphs, and correcting other tests which expected empty text nodes to be there as well.
Change-Id: I414d061cdd494b8023f14e944eda2910a4dab0d4
Extension-specific types are RDFa types (or type regexes) that are
registered with the ModelRegistry separately. If an element has a type
that is extension-specific, then that element can only be matched by a
rule that asserts one of its extension-specific types.
For MediaWiki, we would call
ve.dm.modelRegistry.registerExtensionSpecificType(/^mw:/ ) .
So then an element like <span typeof="mw:foobar"> would either match a
rule specifically for mw:foobar, if one exists, or no rule at all; even
the rule for <span> would not match. The consequence of this is that
elements with unrecognized mw:-prefixed RDFa types are alienated.
Change-Id: Ia8ab1fe5dffb9f813689324372a168e8e4a3e0bc
ModelRegistry registers both annotations and nodes, and performs
matching on both at the same time. It also registers annotations with
the AnnotationFactory, and nodes with the NodeFactory.
Change-Id: I5e68e506a0e573cc0afe6304ccea058ffc20d1c8
Add static properties for matching, data<->DOM conversion, and name. Use
matchTagNames, toDataElement and toDOMElement. name isn't used yet.
Change-Id: I5e7df3303bbd65e6968e931b568c23d76003a9a4
* Made method descriptions imperative: "Do this" rather than "Does this"
* Changed use of "this object" to "the object" in method documentation
* Added missing documentation
* Fixed incorrect documentation
* Fixed incorrect debug method names (as in those VeDmClassName tags we add to functions so they make sense when dumped into in the console)
* Normalized use of package names throughout
* Normalized class descriptions
* Removed incorrect @abstract tags
* Added missing @method tags
* Lots of other minor cleanup
Change-Id: I4ea66a2dd107613e2ea3a5f56ff54d675d72957e
Some of these stubs didn't inherit Node at all. Made them all inherit
LeafNode because their rules specify they can't have children.
Change-Id: If4afc8de350f67ee78a41307c426ec2aceeb884f
See CODING.md for how to run it.
Mistakes fixed:
* Warning: Unknown type function
-> Function
* Warning: Unknown type DOMElement
-> HTMLElement
* Warning: Unknown type DOM Node
-> HTMLElement
* Warning: Unknown type Integer
-> Mixed
* Warning: Unknown type Command
-> ve.Command
* Warning: Unknown type any
-> number
* Warning: Unknown type ve.Transaction
-> ve.dm.Transaction
* Warning: Unknown type ve.dm.AnnotationSet
-> ve.AnnotationSet
* Warning: Unknown type false
-> boolean
* Warning: Unknown type ve.dm.AlienNode
ve.dm doesn't have a generic AlienNode like ve.ce
-> Unknown type ve.dm.AlienInlineNode|ve.dm.AlienBlockNode
* Warning: Unknown type ve.ve.Surface
-> ve.ce.Surface
* ve.example.lookupNode:
-> Last @param should be @return
* ve.dm.Transaction.prototype.pushReplace:
-> @param {Array] should be @param {Array}
* Warning: ve.BranchNode.js:27: {@link ve.Node#hasChildren} links to non-existing member
-> (removed)
* Warning: ve.LeafNode.js:21: {@link ve.Node#hasChildren} links to non-existing member
-> (removed)
Differences fixed:
* Variadic arguments are like @param {Type...} [name]
instead of @param {Type} [name...]
* Convert all file headers from /** to /*! because JSDuck tries
to parse all /** blocks and fails to parse with all sorts of
errors for "Global property", "Unnamed property", and
"Duplicate property".
Find: \/\*\*([^@]+)(@copyright)
Replace: /*!$1$2
* Indented blocks are considered code examples.
A few methods had documentation with numbered lists that were
indented, which have now been updated to not be intended.
* The free-form text descriptions are parsed with Markdown,
which requires lists to be separated from paragraphs by an
empty line.
And we should use `backticks` instead of {braces} for inline
code in text paragraphs.
* Doc blocks for classes and their constructor have to be
in the correct order (@constructor, @param, @return must be
before @class, @abstract, @extends etc.)
* `@extends Class` must not have Class {wrapped}
* @throws must start with a {Type}
* @example means something else. It is used for an inline demo
iframe, not code block. For that simply indent with spaces.
* @member means something else.
Non-function properties are marked with @property, not @member.
* To create a link to a class or member, in most cases the name
is enough to create a link. E.g. Foo, Foo.bar, Foo.bar#quux,
where a hash stands for "instance member", so Foo.bar#quux,
links to Foo.bar.prototype.quux (the is not supported, as
"prototype" is considered an implementation detail, it only
indexes class name and method name).
If the magic linker doesn't work for some case, the
verbose syntax is {@link #target label}.
* @property can't have sub-properties (nested @param and @return
values are supported, only @static @property can't be nested).
We only have one case of this, which can be worked around by
moving those in a new virtual class. The code is unaltered
(only moved down so that it isn't with the scope of the main
@class block). ve.dm.TransactionProcessor.processors.
New:
* @mixins: Classes mixed into the current class.
* @event: Events that can be emitted by a class. These are also
inherited by subclasses. (+ @param, @return and @preventable).
So ve.Node#event-attach is inherited to ve.dm.BreakNode,
just like @method is.
* @singleton: Plain objects such as ve, ve.dm, ve.ce were missing
documentation causing a tree error. Documented those as a
JSDuck singleton, which they but just weren't documented yet.
NB: Members of @singleton don't need @static (if present,
triggers a compiler warning).
* @chainable: Shorthand for "@return this". We were using
"@return {classname}" which is ambiguous (returns the same
instance or another instance?), @chainable is specifically
for "@return this". Creates proper labels in the generated
HTML pages.
Removed:
* @mixin: (not to be confused with @mixins). Not supported by
JSDuck. Every class is standalone anyway. Where needed marked
them @class + @abstract instead.
Change-Id: I6a7c9e8ee8f995731bc205d666167874eb2ebe23
This happens when the <span> is the start of unwrapped content. The
converter logic to look at the tag name in wrapping mode doesn't kick in
because we're not yet in wrapping mode at that point.
The core issue was that previously, we relied on the document
structure/state to choose between alienBlock and alienInline, and only
used the tag name where the document structure was ambiguous (wrapping).
Changed this to be the other way around: we now rely primarily on the
tag name, and if that doesn't match what we expect based on the document
structure, we work around that if possible. Specifically:
* inline tag in our wrapper --> inline alien
* block tag in our wrapper --> close wrapper, block alien
* inline tag in wrapper that's not ours --> inline alien
* block tag in wrapper that's not ours --> *inline* alien
* inline tag in structural location --> open wrapper, inline alien
* block tag in structural location --> block alien
* inline tag in content location --> inline alien
* block tag in content location --> *inline* alien
only in the fourth and the last case do we need to use the "wrong" alien type to
preserve document validity, and it will always be inline where block was
expected, which should reduce UI issues.
The condensed version of the above, which is used in the code, is:
* If in a non-wrapper content location, use inline
* If in a wrapper that's not ours, use inline
* Otherwise, decide based on tag name
* Open or close wrapper if needed
ve.dm.Converter:
* Replace isInline logic in createAlien() with the above
* Factor out code to start wrapping (was duplicated) into startWrapping()
* Call startWrapping() if createAlien() returns an alienInline and we're
in a structural location
Tests:
* Add test cases with aliens at the start and end of unwrapped content
** The first one failed prior to these changes and now passes, the
second one was already passing
* Fix about group test case, was exhibiting the bug that this commit fixes
Change-Id: I657aa0ff5bc2b57cd48ef8a99c8ca930936c03b8
I noticed this bug on [[List of Presidents of the United States]]. When
there's HTML that looks like "<td>Foo\n<meta/></td>", the converter will
collect the newline in wrappedWhitespace, then attempt to splice it out
and store it in internal data. But instead, it ends up splicing out the
/metaBlock element, which causes strange unbalanced input, which causes
an empty table in the node tree.
Change-Id: I79ed2fa9a834cc42759c7d21250d8842f563d38f
ve.ce.Surface
* Switched to using getSlice instead of getData in copy and paste handlers
* Added try/catch which attempts to build a transaction with the unbalanced data first, but falls back on the balanced data otherwise
ve.dm.*Node
* Added default style attributes (now used by ve.dm.NodeFactory)
ve.dm.Document
* Fixed bugs in fixupInsertions where parentType was being set with an object rather than a string
* Made use of getDataElement
* Added adoption capability so that inserting a</h1><p>b into <p>c[cursor]d</p> results in <p>ca</p><p>bd</p> rather than throwing an exception
* Renamed getBalancedData to getSlice, now retuning a ve.dm.DocumentSlice object
ve.dm.DocumentSlice
* Introduced new container for balanced data and a range of the original context - useful for copy/paste
ve.dm.NodeFactory
* Added getDataElement method, which uses default attributes to create a boilerplate version of a data element
ve.dm.Document.test
* Updated getBalancedData test to be a getSlice test
demos/ve/index, VisualEditor, test/index
* Added references to ve.dm.DocumentSlice
Change-Id: Id9269a29e51ca213508de8f155d3feec5e5b0774
The converter was misbehaving when handling <p>s inside <span>s. This
can't be expressed in the linmod, but it would try to anyway. <span><p>
would result in too many paragraph closing elements, leading to an
exception in ve.dm.Document complaining about unbalanced input.
<span>\n<p> would result in an exception in the converter itself while
trying to perform whitespace preservation on the newline.
This change makes the converter detect these scenarios and alienate the
offending node. So <span><p>Foo</p></span> converts to a wrapper
paragraph containing an alienInline whose HTML is "<p>Foo</p>" and which
is annotated with a TextStyleSpanAnnotation.
ve.dm.Converter.getDomFromData():
* Change the criteria for alienBlock vs alienInline
** Only infer from the node type if we're in wrapping mode AND we're at
the same level where the wrapping started (wrappingIsOurs). If the
latter isn't the case, we can't split the wrapper in the block case
because we're at the wrong level.
** Use alienInline not only if the branch is a content branch, but also
if there are active annotations. This catches e.g. <li><b><p>
(and generally <span><p> on the top level).
* Before converting a child element, check that the child isn't "bad".
Bad children are non-content children in content branches, and
non-content children encountered within a wrapper that we can't split.
Only good children are converted, and bad children are alienated (cue
Santa/Sinterklaas jokes).
* Add childIsContent and rename branchIsContent to branchHasContent
Change-Id: If420ae80ab0777424a9a5517335ef9d0170e87ae
HTML DOM has annoying behavior for <pre>s where .innerHTML eats the
first newline in a <pre>. Work around this by explicitly adding a
newline in the data->DOM converter if the <pre> already contained a
newline.
There is a separate bug in Parsoid that causes the newline to be lost
anyway, filed as bug 42666
Change-Id: Ia26cd4a4c61afbe439b0562deb7f24ee8b8147d7
ve.ce.Surface
* Added ve.ce.Surface.adjustCursor, which replaces repetitive and buggy code that was handling left and right arrow key presses
* New method only affects the selection target, so it won't collapse the selection on you - this was what caused bug 42401
* Made hasSlugAtOffset() actually return a boolean
ve.dm.Document
* Fixed turn-around issue in ve.dm.Document.getRelativeOffset - if the offset is already valid and we can't move in the direction we want, we should just leave it be, not turn around
* Since this method was being used by ve.ce.Surface to correct the cursor position on arrow key presses, it was causing the strange cursor jumping when you pressed an arrow key while at the edge of a document
ve.dm.SurfaceFragment
* Fixed typo where getAnnotationRangeFromSelection was preserving selection direction, but checking the wrong properties
ve.dm.Document.test
* Added tests that verify turn-around issue is fixed
Change-Id: Iba55cfc3d531e7d1333b78c94912ff22179aace8
Was broken both on the way in and on the way out.
* Move alien restoration (data->DOM) out of the main getDomFromData()
function and into getDomElementFromDataElement(). This means the
comment about District 9 is gone (sniff), but moving this here ensures
all code paths hit it (previously, it was assumed annotated nodes
could never be aliens).
* In the DOM->data converter, add annotation application to
getDataElementFromDomElement() (for content nodes) and createAlien()
(for aliens). Previously, these nodes would not get annotations.
** ve.AnnotationSet doesn't have a constructor that takes an array, we
should fix that.
Change-Id: I65f8e9a322111ca3af275bf9997b0b1e7ee93769
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
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
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
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
<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
* 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
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
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
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
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
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 s for spaces).
Change-Id: I738a750c91a4ca4836c485e282865bb7525bf30a
* 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
* 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
* 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
* 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
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
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
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
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
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
* 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
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
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
* 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
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
[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
This will cause ve.dm.SurfaceFragment.prototype.insertContent() to place
the selection after the insertion as well.
Change-Id: Ifa7e627daceb90408422eb58c110d475f34ba1e2
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
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
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
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
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
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
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
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
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
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
* 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
Stack traces, line numbers, etc. All the approaches I've seen are bad hacks. This is the best way to go.
Change-Id: Ib12e9d2ecfe610bcc89d046005e35cc13efa3d99
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
* 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
This just cleans up a few places where single quotes should have been used instead of double quotes.
Change-Id: I6c53652e71ab96842ed5bb41fb1e0b8c923eb25d
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
'''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
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
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
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
