Divec has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/80689


Change subject: Use native left/right char cursoring in text nodes
......................................................................

Use native left/right char cursoring in text nodes

modules/ve/ce/ve.ce.Surface.js
* Native left/right cursoring inside a text node (or consecutive text nodes)
* Document the purpose of handleLeftOrRightArrowKey

modules/ve/ve.js
* Revert cluster-aware splitting to trivial javascript code unit splitting.
* Rewrite ve.splitClusters as a trivial compatibility method (needs removing 
soon).

demos/ve/pages/minimal.html
demos/ve/pages/multibyte.html
demos/ve/pages/unicode.html
* replace file with more precise tests

modules/ve/test/ve.test.js
* Remove test of grapheme-based splitting (no longer used)

Change-Id: Ife34c87ebe40bc1689298b592eec5c0cdc2f7589
---
A demos/ve/pages/minimal.html
D demos/ve/pages/multibyte.html
A demos/ve/pages/unicode.html
M modules/ve/ce/ve.ce.Surface.js
M modules/ve/test/ve.test.js
M modules/ve/ve.js
6 files changed, 68 insertions(+), 62 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/VisualEditor 
refs/changes/89/80689/1

diff --git a/demos/ve/pages/minimal.html b/demos/ve/pages/minimal.html
new file mode 100644
index 0000000..ca713e3
--- /dev/null
+++ b/demos/ve/pages/minimal.html
@@ -0,0 +1 @@
+<h1>X</h1>
diff --git a/demos/ve/pages/multibyte.html b/demos/ve/pages/multibyte.html
deleted file mode 100644
index 979b312..0000000
--- a/demos/ve/pages/multibyte.html
+++ /dev/null
@@ -1,6 +0,0 @@
-<p>12𨋢456789𨋢bc</p>
-<p>「𨋢」字響<tt>香港</tt>衍生出好多新詞,好似:𨋢<tt>香港</tt> abc</p>
-<p>abc</p>
-<p>one c̀ombining accent</p>
-<p>two ç̀ombining accents</p>
-<p>def</p>
\ No newline at end of file
diff --git a/demos/ve/pages/unicode.html b/demos/ve/pages/unicode.html
new file mode 100644
index 0000000..cc8142a
--- /dev/null
+++ b/demos/ve/pages/unicode.html
@@ -0,0 +1,4 @@
+<h1>Above FFFF: 12𨋢456789𨋢bc</h1>
+<h1>Cluster test: ക്ത്ര faç̀on</h1>
+<h1>Multiline TT / cursoring test: 「𨋢」係一個<tt>香港字 香港字 香港字 香港字 香港字 香港字 香港字 香港字 
香港字 香港字 香港字 香港字 香港字 香港字 香港字 香港字 香港字 香港字 香港字 香港字 香港字 香港字 香港字 香港字 香港字 香港字 香港字 香港字 
香港字 香港字 香港字 香港字 香港字 香港字 香港字 香港字 香港字 香港字 香港字 </tt> 呢啲瀏覽器:真係好<tt>麻煩</tt>嘅喎!</h1>
+<h1>Bidi test: עב12345רית</h1>
diff --git a/modules/ve/ce/ve.ce.Surface.js b/modules/ve/ce/ve.ce.Surface.js
index cbc5ba1..9131745 100644
--- a/modules/ve/ce/ve.ce.Surface.js
+++ b/modules/ve/ce/ve.ce.Surface.js
@@ -987,6 +987,16 @@
 
 /**
  * @method
+ * At times we can't rely on the browser's native Left/Right handling. For 
instance:
+ * * To prevent cursoring into an image caption
+ * * Or into a template in IE (which does not respect contenteditable=false 
inside contenteditable=true)
+ * * To allow selection of elements (e.g. placing the cursor "on image")
+ * * To prevent the cursor disappearing on some browsers in places like 
<table>|<tr>
+ * * To handle backwards selection in IE (which cannot be set 
programmatically, so needs faking)
+ * * To give more consistency between browsers
+ *
+ * On the other hand, the browser's native handling is positively required to 
handle cursoring
+ * over multi-codepoint Indic grapheme clusters, combining accents etc.
  */
 ve.ce.Surface.prototype.handleLeftOrRightArrowKey = function ( e ) {
        var selection, range, direction;
@@ -994,6 +1004,13 @@
        // As we are not able to handle it programmatically (because we don't 
know at which offsets
        // lines starts and ends) let it happen natively.
        if ( e.metaKey ) {
+               return;
+       }
+
+       // If we're cursoring by character, within a text node, then use native 
browser handling
+       // in order to handle grapheme clusters correctly.
+       if ( ! e.altKey && ! e.ctrlKey && this.willMoveInsideTextNode(
+               e.keyCode === ve.Keys.LEFT ? 'backward' : 'forward' ) ) {
                return;
        }
 
@@ -1020,6 +1037,44 @@
        this.surfaceObserver.start( false, true );
 };
 
+ve.ce.Surface.prototype.willMoveInsideTextNode = function ( direction ) {
+       var range, node, backward = direction === 'backward',
+               sel = rangy.getSelection( this.getElementDocument() );
+       if ( sel.rangeCount === 0 ) {
+               return false;
+       }
+
+       // If moving backwards, get first range; if moving forwards, get last 
range
+       if ( backward ) {
+               range = sel.getRangeAt( 0 );
+               node = range.startContainer;
+       } else {
+               range = sel.getRangeAt( sel.rangeCount - 1 );
+               node = range.endContainer;
+       }
+
+       if ( node.nodeType !== node.TEXT_NODE ) {
+               return false;
+       }
+
+       // Kludge: in Chrome, cursoring can split text nodes in two, so check 
carefully. Note
+       // we can't just normalize() the parent node, because that changes the 
selection state.
+       if ( range.startOffset === 0 && node.previousSibling !== null &&
+               node.previousSibling.nodeType === node.TEXT_NODE ) {
+               return true;
+       }
+       if ( range.endOffset === node.nodeValue.length && node.nextSibling !== 
null &&
+               node.nextSibling.nodeType === node.TEXT_NODE ) {
+               return true;
+       }
+
+       if ( backward ) {
+               return range.startOffset > 0;
+       } else {
+               return range.endOffset < node.nodeValue.length;
+       }
+};
+
 /**
  * @method
  */
diff --git a/modules/ve/test/ve.test.js b/modules/ve/test/ve.test.js
index a48a1d0..9dd2bbc 100644
--- a/modules/ve/test/ve.test.js
+++ b/modules/ve/test/ve.test.js
@@ -444,53 +444,3 @@
                assert.equalDomElement( $( 'body', doc ).get( 0 ), 
expectedBody, cases[key].msg + ' (body)' );
        }
 } );
-
-// ve.splitClusters: Tested upstream (UnicodeJS)
-
-// TODO: ve.isUnattachedCombiningMark
-
-// TODO: ve.getByteOffset
-
-// TODO: ve.getCharacterOffset
-
-QUnit.test( 'graphemeSafeSubstring', function ( assert ) {
-       var i, text = '12\ud860\udee245\ud860\udee2789\ud860\udee2bc', cases = [
-                       {
-                               'msg': 'start and end inside multibyte',
-                               'start': 3,
-                               'end': 12,
-                               'expected': [ 
'\ud860\udee245\ud860\udee2789\ud860\udee2', '45\ud860\udee2789' ]
-                       },
-                       {
-                               'msg': 'start and end next to multibyte',
-                               'start': 4,
-                               'end': 11,
-                               'expected': [ '45\ud860\udee2789', 
'45\ud860\udee2789' ]
-                       },
-                       {
-                               'msg': 'complete string',
-                               'start': 0,
-                               'end': text.length,
-                               'expected': [ text, text ]
-                       },
-                       {
-                               'msg': 'collapsed selection inside multibyte',
-                               'start': 3,
-                               'end': 3,
-                               'expected': [ '\ud860\udee2', '' ]
-                       }
-               ];
-       QUnit.expect( cases.length * 2 );
-       for ( i = 0; i < cases.length; i++ ) {
-               assert.equal(
-                       ve.graphemeSafeSubstring( text, cases[i].start, 
cases[i].end, true ),
-                       cases[i].expected[0],
-                       cases[i].msg + ' (outer)'
-               );
-               assert.equal(
-                       ve.graphemeSafeSubstring( text, cases[i].start, 
cases[i].end, false ),
-                       cases[i].expected[1],
-                       cases[i].msg + ' (inner)'
-               );
-       }
-} );
diff --git a/modules/ve/ve.js b/modules/ve/ve.js
index 566a13b..5340f76 100644
--- a/modules/ve/ve.js
+++ b/modules/ve/ve.js
@@ -578,12 +578,14 @@
                return ve.init.platform.getMessage.apply( ve.init.platform, 
arguments );
        };
 
-    /**
-     * @method
-     * @inheritdoc unicodeJS.graphemebreak#splitClusters
-     * @see unicodeJS.graphemebreak#splitClusters
-     */
-       ve.splitClusters = unicodeJS.graphemebreak.splitClusters;
+       /**
+        * Compatibility method. We no longer split into clusters at this level.
+        *
+        * TODO: strip out calls to splitClusters then delete this method.
+        */
+       ve.splitClusters = function ( text ) {
+               return text.split( '' );
+       };
 
        /**
         * Determine if the text consists of only unattached combining marks.

-- 
To view, visit https://gerrit.wikimedia.org/r/80689
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ife34c87ebe40bc1689298b592eec5c0cdc2f7589
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Divec <[email protected]>

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to