Added check for the existence of the surface before the save dialog is hidden. This oversight caused errors when there was a load error and the user clicked cancel.
Change-Id: I3b6d8d4d9f4c81ffcbd7996d6b17ac04f33bf6d7
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
The {{coords}} template is correctly rendered as an inlineAlien, which previously did not have shields. However, the template has a deeply nested span that is positioned and floated. The fix is to shift shielding logic up to AlienNode (from AlienBlockNode) and to show phantoms on all shields (both block and inline). I've also updated the aliens demo to exhibit this corner case.
Change-Id: Ifce60c7762c0ead5c6fe29d6eabf601c1565cbfa
Although $.toJSON optimises heavily for modern browsers (it
becomes a direct reference to JSON.stringify), we still load the
extra plugin.
JSON is specified as part of ECMAScript 5, but most browsers
supported this one before they supported the rest of ES5.
http://caniuse.com/#search=JSON
Cut off for native JSON is IE7, Firefox 3.0 (3.6 supports it) and
Safari 3. Not any of our concern as VE will most likely never
support those (certainly not at this point in time, and less
likely as time goes on).
Change-Id: I4e8f26ac94763fa38d29e41264de0247f53a21e5
en.wikipedia.org has a template gala for edit notices with a whole
bunch of html framework outputted by default from
MediaWiki:Editnotice-0 (even if their underlying system has no
matches for the current page).
In the core editor from EditPage.php this isn't a problem as the
element is just idling hidden above the editor.
In the case of VisualEditor (where we have a custom delivery for
the edit notices) we don't want to say "1 notice available" on
every page, so we need to be smart and quickly walk the dom of the
notice, filter out invisible nodes, and if the resulting nodes
have no contents, ignore the notice all together.
Change-Id: I65447da8b88a9bae9c24ff155544ff66b3fe9100
The rules have changed, these constraints are no longer needed
and actually causing the table.diff to ignore the restrictions
and bump out of the container beyond the width of the window.
This restores the condition under which table.diff is displayed
by core EditPage.php.
Follows-up I5f59482f4db.
Change-Id: I456644bf14efab7dac351818d5c3bb69b9b3c993
Basically saveDialog-body now has three slides:
* review
* report
* save
There is a viewPage.swapSaveDialog method that is called like this
viewPage.swapSaveDialog( 'review' );
initially called from showSaveDialog.
Misc:
* Replaced more reused core message with a ve ones:
savearticle => visualeditor-savedialog-label-save
showdiff => (removed)
* Removed min-height from saveDialog. When slide-review is
load, it is very short and there shouldn't be 10em's of
whitespace below the loader + buttons.
To avoid problems with diff cache being cleared while looking
at the save dialog, we lock and unlock the surface.
We could later remove this as and optimise it as feature, but for
now this fixes a bug.
Change-Id: I5f59482f4db16264014720b199d7652843c36108
-Prevents previously inspected link suggestions from showing prior
to rebuild.
-Ability for onInput to be executed immediately.
-Removed duplicated logic from open method.
-No longer calling onInput on enter key.
Change-Id: I11792b03ec01cc1929009fb76089adcc86c97629
Drop "-down" class in favour of :active CSS pseudo-class.
* http://kimblim.dk/css-tests/selectors/
* http://www.quirksmode.org/css/contents.html
* http://caniuse.com/#search=css%202.1
Rename methods consistently to enableFoo and disableFoo.
Use mw.notify instead of mw.util.jsMessage (the latter takes a
string of html and parses it, instead of taking text).
Also, jsMessage is obsolete (legacy wrapper around mw.notify).
Rename slide-main to slide-save in preparation of other slides
and introduction of multi-step save dialog.
Re-introduce "disabled" state (not css, but actual state) so that
pressing "Show changes" multiple times will not spawn multiple
async operations. The save function guards against this by keeping
a static property in the internal helper method, but there are
still some operations happening between the button and that method
this should be guarded in the UI instead. Follows-up I67819168.
Change-Id: Idf0331377e6be814ccae853228d6159b4ad5a159
In the change: I6781916822b4173ba906b9bc4c341219fbf10d82
71b97ec45b was reversed when I submitted 1789545284
Change-Id: I6cc59a27817bfbe55c7cbf45bba746a4bd6c375f
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
icons, comment.*, ve.ui.Icons-*
* Added comment icon
ve.init.mw.ViewPageTarget
* Refactored editNoticeButton into being just a tool
* Added feedback tool, which shares the editNoticeButton code
* Added feedback object construction in init constructor
* Added launching code to feedback tool
VisualEditor.i18n
* Renamed editnotices-button message to editnotices-tool to match changes in code
* Added feedback tool message
VisualEditor
* Added dependency on mediawiki.feedback
* Updated changed message key
* Added reference to new message
Change-Id: I813c89a505386a9b3206bfbcb176016e28a592cb
* Made buttons use shared CSS code
* Stopped using <button> elements and got all browsers looking the same
* Fixed focus on text area issue - had to let the stack clear first
Change-Id: I6781916822b4173ba906b9bc4c341219fbf10d82
This was removed in I0873d906c058203b83b8d4bbe5a4b274f05a26fd because I had fixed another cause of the recursion - this is still needed however because the following still happens:
1. Link inspector is opened away from a word, so it goes into a mode that will insert text based on the chosen target
2. The link inspector is closed, causing a context change when the new annotated text is inserted
3. The context change event triggers the link inspector to close, leading us back to step 2
4. Browser dies in infinite loop hell
We may be able to fix this in a different way, like performing the document changes after the inspector has already closed by running it in a timeout. In any case, this works for now, and we can improve the design of this area of the code later on.
Change-Id: I7f3c17dad08521bb1ae7c2a1e8a4e21e2c4ba210
In the event the page explodes from a huge js error, prevent
the iframe from reloading to slightly reduce the confusion of the
user.
Change-Id: I0c88678437f0bad994fa97f4df63d8b288f69e7a
-Added reset and hide calls to deactivate to prevent problems
with the save dialog if the user exits to the view.
-Remove unneeded teardown routines from cancel.
-Calling deactivate without override.
Change-Id: I888019146186fb7cbc1ee2b8efee9a0c45286c23
ve.Range
* Rewrote truncate so that it works as expected, truncation should always reduce the length using the start/end values, not the from/to values
ve.ui.Inspector
* Added a comment about where the name argument to onBeforeInspectorOpen comes from, since it's a little bit confusing on first read (and I wrote it!)
* Calling onInitialize, onOpen and onClose methods directly, since we need to control the before or after-ness of listeners getting in there and doing their stuff - plus it's more direct
* Removed onRemove stub, which is never actually called
* Added before/after versions of initialize, open and close events
* Got rid of recursion guard since we don't need it anymore thanks to changes made in ve.dm.Surface (see below)
ve.ui.Context
* Updated event names to deal with new before/after naming of initialize, open and close events
* Removed fade-in logic since fading in doesn't even work anymore - since now we now annotate first, then open the inspector, the menu will actually exist and be open when we open the inspector even though you don't see it because it's quickly obscured
ve.ui.LinkInspector
* Made fragments non-auto selecting, in the case of onInitialize we actually call select(), which is silly since we were using an auto-selecting fragment - it's clear that this was a mistake
ve.dm.Surface
* Moved locking (polling stop and start) to the far outside edges of the change method
* I need a lot of eyes and testing on this change, it seems OK to me, but I'm suspicious that it may have side effects
* What was happening is that selection changes were being applied and then the poll was picking them up, and then the selection was coming in again as a change, but it wasn't a change at all, it was just feedback - this change event was then closing the inspector the instant it was opened - the odd part was that this only occurred when you selected backwards, which seems to be caused by the range being normalized, so it looked like a new selection even though it wasn't
ve.dm.Document
* trimOuterSpace from Range didn't consider annotated spaces to be spaces, by using the [0] trick (first character of a plain text character string or first element in an annotated character's array both are the character's value - but elements don't have a property named '0' so it skips those safely as well) we can always get the right value for comparison
Change-Id: I0873d906c058203b83b8d4bbe5a4b274f05a26fd
Solution: do not call update (temporary solution to prevent error - however content will never be at offset 0 - so it should never hurt)
Change-Id: I9ed6f9fbea23d5e2e55c56f8483321442049bd51
Turned saveDialog-body into slide-based swapper.
Moved footer into saveDiaog-body so that the license text doesn't
stay under the diff-slide (and move body bottom padding to foot
top)
Wrapped buttons and title in a saveDialog-header and converted
closeButton from absolutely positioned to a floated layout.
This way the title doesn't need to be repositioned but will scooch
over if the prevButton gets shown/hidden.
Update API "diff" action to include table wrapper and table
header. Without it the mediawiki CSS for diff doesn't work
properly (needs colgroups for proper width of the "-" and "+" column etc)
Renamed -saving class to -disabled for consistency.
Set prop.disabled to really lock/unlock buttons, not just visual
(otherwise the click handlers are still triggered on click, can
potentially cause actions to be triggered when not expected)
Using a ve message for "Show your changes" title instead of
re-using core tooltip-savepage in a different context.
Diff slide triggers "auto width" on dialog (inline undo of width: 29em), keeping min-width, to allow it to expand as wide as needed.
Functions that I copied as base for onShowChanges and
onShowChangeError had some incorrect argument descriptions. Fixed
in both.
Note:
* Pass function to .off(), so that only that one is unbound
instead of any "resize" handler on the page (by other extensions
or gadgets or core)
* NB: ve.bind ($.proxy) preserves internal guid, so that $.Event
can find the bound function by the original reference.
* keydown has an anonymous function, should either moved to
prototype or namespaced, did latter for now, save enough and
better than destructive .off('keydown')
Change-Id: I9d05ef6e3e2461bdcf363232f7b0fbad5e24f506
We got lucky - the fact that our alien styling choice required phantom overlays had the nice side-effect of preventing the ability to focus the aliens. Therefore, as soon as we started ignoring IE to deliver nice alien handling for standards-compliant browsers, we inadvertently solved(?) the IE alien problem. I've touched up the styling to look nice in IE 9 (solid green phantoms) and IE 10 (stripes). Shield GIF switched to PNG because IE 10 renders the transparent GIF as solid red. Yeah, red.
Change-Id: I3bc69acba9ed883a823cdf722117b90966bc332b
This solves 2 issues:
1. Paste scrolls to the top of the editable box because we temporarily moved the focus to the paste container, and then moved it back. We solve this by saving/restoring the scroll position.
2. Scroll position adjusts vertically when pasting (using key repeat) because we handle pastes with a setTimeout and multiple paste jobs will pile up before they can be finished. We solve this by disallowing paste until we are done handling the previous one. In the case of key repeat this only is observable as slowing the repeat rate.
Thank you Christian for helping me solve this the old fashioned way - by sacrificing features you don't really need!
Change-Id: I75bcf9843b57cdff0db9b6dea62d66d4d76ac011
ve.dm.Document
* 2 of the 3 paths of getSlice still returned arrays instead of ve.dm.DocumentSlice objects
ve.ce.Surface
* Translate the range using the insertion transaction and truncate it, so the cursor ends up just after the pasted content
Change-Id: If7bae5e254ec84a847c1d3527f74d9c09c2d82b4
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 fix for bug 40339 supposedly modified when we emit contextChange events so that when the node changed, even if the context was the same, we consider it a context change (such as changing the heading level).
Unfortunately I was mentally absent when I wrote the patch and all it actually does it emit more select events.
Basically cb4877b0d0 - which does indeed fix the bug - doesn't do what it's commit message describes, but this fixes it.
Change-Id: I99d74f9ab0ddec15df41320389fe83de9b8b8d1e
* Decode titles we get from Parsoid
* Encode titles we give to Parsoid
* Preserve the original encoded title if nothing meaningful changed (a.k.a. do not normalize unless we must)
Change-Id: If5d22e88904d6b2c438caac403ac2d78d440b017
* Don't show at all if user isn't logged in
* Use "watch pages I create" and "watch pages I create" prefs
* If the user is already watching it, use that
Also updated relevant onMakeGlobalVariablesScript hook,
it was using old globals still, the hook has context as
of MediaWiki 1.19.
Change-Id: Ic3daf32505a745b3cccd0663a03bbf7f3885be84
Since model.change now starts the surfaceObserver, a problem manifested where the surfaceObserver was observing the $('#paste') element (used for capturing and sanitizing pasted content from external sources). Simply switching the order of the transaction and the documentNode focus fixed the problem.
Change-Id: I1b542b546d7f3d97e2e954ca4b51ca74232b2877
* Reusing deactivate method to recover from load error
* Made deactivate resilient to some properties not being set yet (so we can call it on load error)
* Restoring save dialog state after save error
Change-Id: I6a697dc6bddeebecf4e2ab26805bee9f3754c714
Logic taken from EditPage::getCopyrightWarning, but couldn't use
it directly because it doesn't give the message keys and wraps
the html.
Using the same logic and running the same hook, we'll get the same
message keys (and message parameters therefore) as EditPage would.
Also fixed these bugs (as they were more prominent now):
* Use Message::parse() instead of Message::plain()
* Set inLanguage properly instead of using the default
(EditPage is user-localised, including this message, just like
the rest of VE).
Fixes bug 42764: minoredit/watchthis should be in user-language.
Change-Id: I84fee641162cdeed290092e56fb0e1d2562d833d
