Commit graph

3701 commits

Author SHA1 Message Date
jenkins-bot d03aaa102c Merge changes I7b0e3fa4,I559232f3
* changes:
  ve.init.mw.ViewPageTarget: Put the Edit source link in the visible tab area
  ve.init.mw.ViewPageTarget: Switch tabLayout from 'add' to 'replace'
2013-04-18 21:02:23 +00:00
jenkins-bot eac3cb99e0 Merge "Added inline comment explaining not obvious purpose of code" 2013-04-18 20:59:25 +00:00
jenkins-bot 22622c922b Merge "Death and/or destruction" 2013-04-18 20:58:19 +00:00
Trevor Parscal 2419f7638c Death and/or destruction
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
2013-04-18 13:56:20 -07:00
Inez Korczyński d809dc7e37 Added inline comment explaining not obvious purpose of code
Change-Id: Ib9ea1307ebe58c03abd00dcf8b96681d6901a3d4
2013-04-18 13:39:56 -07:00
jenkins-bot d54cf4c131 Merge "Transactions to store metadata merge info when present" 2013-04-18 20:04:51 +00:00
jenkins-bot 1bfc9594b3 Merge "Avoid rendering content of ve.ce.ContentBranchNode multiple times when only one transaction is applied" 2013-04-18 18:58:42 +00:00
jenkins-bot 16d164ed6d Merge "Delete spurious normalizedHtml" 2013-04-18 15:32:53 +00:00
jenkins-bot 289116c493 Merge "When we say originalHtml, we'd better mean it" 2013-04-18 15:27:25 +00:00
Catrope cdba5f2800 When we say originalHtml, we'd better mean it
Gabriel noticed whitespace diffs in the problem reports. These were
caused by browsers' .innerHTML being broken on <pre>s. We compensate
for this in the converter, but not when generating originalHtml.

However, a variable called originalHtml really shouldn't be generated,
it should just literally be the original, unprocessed HTML. This
does mean it includes the doctype, <html> and <head> which aren't
included in editedHtml (that one's just the contents of the <body>), but
that's much easier to deal with on Parsoid's end than random newline
diffs all over the place.

Change-Id: I8e66cb79887f49f84114ab6b4d0e0d24aea744b6
2013-04-18 08:09:50 -07:00
jenkins-bot 17a93e96ec Merge "Serialize alienated context-sensitive nodes correctly" 2013-04-18 11:10:37 +00:00
jenkins-bot 52a666956c Merge "Fix a weird whitespace stripping bug" 2013-04-18 10:56:17 +00:00
Catrope 42c1aa10cd Serialize alienated context-sensitive nodes correctly
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
2013-04-17 18:26:54 -07:00
Catrope 50e14d42d4 Delete spurious normalizedHtml
Was equal to the HTML before conversion, so not needed

Change-Id: I48e3b3d48334562f605bccfddc5f2aa7cf883087
2013-04-17 17:54:35 -07:00
jenkins-bot 7fe249f034 Merge "Partly fix the "Report problem" button" 2013-04-18 00:17:27 +00:00
Catrope 2f1ee49213 Fix a weird whitespace stripping bug
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
2013-04-17 17:08:47 -07:00
Catrope 6c1e4cfdd2 Partly fix the "Report problem" button
This was broken ever since the introduction of IndexValueStore, because
the call to getDataFromDom() wasn't updated, so it crashed with a "doc
is undefined" error. Fixing part of this by passing in a new IVStore.

The data that is transmitted over the wire still has indices with no way
to find out what the corresponding annotations are. It needs to be fully
expanded but there's no way to do this in DM quite yet.

Bug: 47319
Change-Id: I761523d22e51ac560e37ae991d01a6b84224ca40
2013-04-17 23:04:56 +00:00
Catrope a20ec306a5 Fix missing global in 116321afe5
Change-Id: Ia76ce0cdd3fed3de1829ae397f1ab3cf3686d8dc
2013-04-17 15:29:58 -07:00
Inez Korczyński e786c5c2f0 Avoid rendering content of ve.ce.ContentBranchNode multiple times when only one transaction is applied
Change-Id: I940bb96b670afb7ab3de524919b56912d027a2ef
2013-04-17 12:16:04 -07:00
Catrope eab968d7f0 ve.init.mw.ViewPageTarget: Put the Edit source link in the visible tab area
Instead of in the collapsed section.

Change-Id: I7b0e3fa4e35ce6135e3a47360da51fd8e0231ca1
2013-04-17 18:15:43 +00:00
Catrope 116321afe5 Make the "Report problem" POST URL configurable
Defaults to "$wgVisualEditorParsoidURL/_bugs/"

Change-Id: I4847d54b0effe433d5d179d91fdf361c249515ad
2013-04-17 17:05:58 +00:00
Catrope 04516bb02e Whitespace preservation was broken after the first run
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
2013-04-17 11:28:05 +00:00
Catrope 9a98d8bce4 ve.init.mw.ViewPageTarget: Switch tabLayout from 'add' to 'replace'
This makes the Edit tab point to the VisualEditor, and adds a
"Edit source" link to the p-cactions menu.

Bug: 46872
Change-Id: I559232f3d8e42df0d45311b65df8c30425b1a368
2013-04-17 10:25:36 +00:00
jenkins-bot 1b072d2e0c Merge "Inside ve.ce.Document.getRelativeOffset fallback to 'character' mode if 'word' mode returns the same offset as was passed originally" 2013-04-16 22:54:27 +00:00
Ed Sanders ff7b8a2591 Transactions to store metadata merge info when present
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
2013-04-16 22:26:56 +01:00
Inez Korczyński 9d5b9525d6 Inside ve.ce.Document.getRelativeOffset fallback to 'character' mode if 'word' mode returns the same offset as was passed originally
Change-Id: Ia3de5d5c1f37f17161fb1c8c9a284c868bb4a5fb
2013-04-16 13:12:25 -07:00
Translation updater bot cec856032a Localisation updates from http://translatewiki.net.
Change-Id: If48c12717c23a0e05122f9912118eddd1b4f03c9
2013-04-16 19:47:52 +00:00
jenkins-bot 11f4bbbb9d Merge "There was no check if anchorNode of selection range is inside an element with CSS class ve-ce-slug." 2013-04-16 18:35:36 +00:00
jenkins-bot 5000cbaa80 Merge "Make returning the same offset the least preferred option in ve.ce.Document.getRelativeOffset" 2013-04-16 18:26:54 +00:00
jenkins-bot a5e49f8716 Merge "Fix annotation breakage" 2013-04-16 01:03:48 +00:00
Catrope 383a669f7c Fix annotation breakage
Inez reported that unitalicizing from the toolbar was broken, because
the toolbar was somehow generating annotations that had .attributes={}
as opposed to .attributes=undefined. Turned out the cause was in the
default value for element in the ve.dm.Model constructor.

Change-Id: I64ea9ef56cd15d1131c1aa23484d7420c95a8225
2013-04-15 17:04:50 -07:00
Timo Tijhof e150011849 docgen: Disable parallel processing, suspected cause of Ruby segfaults
Change-Id: Icdf92b4c0aed27f7bfc5d37c6abeedf60a2b7f27
2013-04-15 22:32:26 +00:00
Inez Korczyński d4e41e2908 Make returning the same offset the least preferred option in ve.ce.Document.getRelativeOffset
Change-Id: If21cdea220b58aec92b8735e92f594d8c16c8f6f
2013-04-15 14:59:04 -07:00
Inez Korczyński 097fd3c1a4 There was no check if anchorNode of selection range is inside an element with CSS class ve-ce-slug.
Adding that check let me simplify logic around a little bit as well.

Bug: 47258

Change-Id: I51bf5e9caecf6a8b4e0f75118a93cfe69a5a6542
2013-04-15 21:40:13 +00:00
Translation updater bot 7613972cbb Localisation updates from http://translatewiki.net.
Change-Id: I43d68d92b1a36f3832881391932bb8b2cc157f8b
2013-04-15 21:05:05 +00:00
jenkins-bot 887465b159 Merge "Filter our zero-coverage updateState nodes" 2013-04-14 02:53:01 +00:00
jenkins-bot 5764026d00 Merge "TransactionProcessor: Fix offset bug" 2013-04-14 02:51:12 +00:00
Trevor Parscal 504a1bafd0 Filter our zero-coverage updateState nodes
Added filter for nodes being passed to update state event - this
ensures that nodes are not being included with zero-length coverage, a
side effect of how selectNodes handles the virtual boundaries of text
nodes.

Change-Id: I6362114b57469b1108da11f94dc345a2bcdfc7cd
2013-04-14 02:50:01 +00:00
Ed Sanders 6dacc54954 Hybridise MWTemplateNode
* 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
2013-04-14 02:34:18 +00:00
jenkins-bot 1b5f4704ff Merge "Have ce's ctrl+arrow functionality use UnicodeJS library" 2013-04-13 08:06:20 +00:00
Timo Tijhof f415b8d747 JSHint: Re-order jshintrc options to split common from project settings
Per I43ace21208 and https://www.mediawiki.org/wiki/CC/JS#Linting

Change-Id: I280d377071ca1244130cd976aeba11d2152ef923
2013-04-13 08:53:11 +02:00
Inez Korczyński 77d9606bde TransactionProcessor: Fix offset bug
Use ve.BranchNode.getNodeFromOffset instead of
ve.dm.Document.getNodeFromOffset so correct nodes will be retrieved to
emit update events to.


Change-Id: Iaf559f0424584a3dde065e548e403c4a53207312
2013-04-12 23:16:51 +00:00
jenkins-bot b9412011cb Merge "Create MWTemplateNode" 2013-04-12 19:06:43 +00:00
Translation updater bot 61e2db4d35 Localisation updates from http://translatewiki.net.
Change-Id: Ifd24a43489b8921e3c1e85f61718351e6c021e07
2013-04-12 18:20:52 +00:00
Ed Sanders 7f6beb9d39 Have ce's ctrl+arrow functionality use UnicodeJS library
Change 57076 implemented the functionality required to skip
words in UnicodeJS. This change simply removes all the existing
code we had and replaces it with this implementation.

Bug: 46794
Change-Id: I6b2700d65476c4d34ba4a01a88382d7af8e736fb
2013-04-12 12:12:53 +01:00
Ed Sanders 01eda7f36a Create MWTemplateNode
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
2013-04-11 23:58:48 +01:00
Catrope 54a232a92b Allow nodes to handle their own children
For data->DOM, this is easy: .toDataElements() can optionally return an
array instead of an object, and that will be treated as the data to
insert. If this happens, the converter won't descend. The node handler
can recursively invoke the converter if it needs to (although I suspect
the current implementation is broken when converting block content in an
inline context).

For DOM->data, this is a bit more complex. The node sets
.static.handlesOwnChildren = true; , which triggers the converter to
pass a data slice rather than a single data element, and not to
descend. The node handler can invoke the converter to recursively
convert DOM subtrees to data.

ve.dm.Converter (data->DOM):
* Renamed createDataElement() to createDataElements()
** .toDataElement() may return element or array, handle this
* Renamed childDataElement to childDataElements, is now an array
* Actually alienate if .toDataElement() returns null
** Shockingly, this claimed to be supported before but wasn't
* Rather than pushing to data, concat to it
** Add closing if needed
* Don't descend if .toDataElement() returned an array of length >1, or
  if the node has .handlesOwnChildren = true

ve.dm.Converter (DOM->data):
* Split getDomSubtreeFromData() and getDomFromData()
* When converting a node that handles its own children, pass in a data
  slice and skip over that data

Change-Id: I196cb4c0895cbf0b428a189adb61b56565573ab3
2013-04-11 22:41:18 +00:00
jenkins-bot 6123f6e952 Merge "Fixes to DM metaList stuff under Roan's guidance." 2013-04-11 22:32:38 +00:00
Rob Moen 1c6522d9e9 Fixes to DM metaList stuff under Roan's guidance.
Splitting this out of https://gerrit.wikimedia.org/r/#/c/58274

Change-Id: Ic90b9714959e1cfb0c18997e2f2b593ed9909990
2013-04-11 14:49:17 -07:00
jenkins-bot 315fa793f4 Merge "Allow hybrids across Model subclasses" 2013-04-11 21:45:59 +00:00