Esanders has uploaded a new change for review.
https://gerrit.wikimedia.org/r/160579
Change subject: Remove getSelection*Client* methods
......................................................................
Remove getSelection*Client* methods
Maintaining two versions for one optimisation is too much work.
Change-Id: I92fdcc24c6114e5e02d5bc5b227b1660d2429e52
---
M src/ce/ve.ce.Surface.js
M src/ui/ve.ui.DesktopContext.js
M src/ui/ve.ui.Toolbar.js
M tests/ce/ve.ce.Surface.test.js
4 files changed, 75 insertions(+), 106 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/VisualEditor/VisualEditor
refs/changes/79/160579/1
diff --git a/src/ce/ve.ce.Surface.js b/src/ce/ve.ce.Surface.js
index dd26b76..04faf41 100644
--- a/src/ce/ve.ce.Surface.js
+++ b/src/ce/ve.ce.Surface.js
@@ -317,27 +317,18 @@
};
/**
- * Get the inline coordinates of the selection range relative to the viewport.
+ * Get the inline coordinates of the selection range relative to the surface.
*
- * Returned coordinates are client coordinates (i.e. relative to the viewport).
- * For coordinates relative to the surface, use
#getSelectionInlineRelativeRects
+ * Returned coordinates are relative to the surface.
*
* @method
* @returns {Object|null} Start and end selection rectangles
*/
-ve.ce.Surface.prototype.getSelectionInlineClientRects = function () {
- var inlineRects, nativeRange, surfaceRect, boundingRect, rtl, x,
collapsedRect;
+ve.ce.Surface.prototype.getSelectionInlineRects = function () {
+ var inlineRects, nativeRange, surfaceRect, focusNodeRect, rtl, x,
collapsedRect;
+
if ( this.focusedNode ) {
- inlineRects = this.focusedNode.getInlineRects();
- surfaceRect = this.getSurface().getBoundingClientRect();
- if ( !inlineRects || !surfaceRect ) {
- return null;
- }
- // Convert surface relative coords to client coords
- return {
- start: ve.translateRect( inlineRects.start,
surfaceRect.left, surfaceRect.top ),
- end: ve.translateRect( inlineRects.end,
surfaceRect.left, surfaceRect.top )
- };
+ return this.focusedNode.getInlineRects();
}
nativeRange = this.getNativeRange();
@@ -349,29 +340,29 @@
// * in Firefox on page load when the address bar is still focused
// * in empty paragraphs
try {
- return ve.getStartAndEndRects( nativeRange.getClientRects() );
+ inlineRects = ve.getStartAndEndRects(
nativeRange.getClientRects() );
} catch ( e ) {
// When possible, pretend the cursor is the left/right border
of the node
// (depending on directionality) as a fallback.
if ( this.nativeSelection.focusNode &&
this.nativeSelection.focusNode.nodeType === Node.ELEMENT_NODE ) {
// We would use getBoundingClientRect(), but in iOS7
that's relative to the
// document rather than to the viewport
- boundingRect =
this.nativeSelection.focusNode.getClientRects()[0];
- if ( !boundingRect ) {
+ focusNodeRect =
this.nativeSelection.focusNode.getClientRects()[0];
+ if ( !focusNodeRect ) {
// FF can return null when focusNode is
invisible
return null;
}
rtl = this.getModel().getDocument().getDir() === 'rtl';
- x = rtl ? boundingRect.right : boundingRect.left;
+ x = rtl ? focusNodeRect.right : focusNodeRect.left;
collapsedRect = {
- top: boundingRect.top,
- bottom: boundingRect.bottom,
+ top: focusNodeRect.top,
+ bottom: focusNodeRect.bottom,
left: x,
right: x,
width: 0,
- height: boundingRect.height
+ height: focusNodeRect.height
};
- return {
+ inlineRects = {
start: collapsedRect,
end: collapsedRect
};
@@ -379,75 +370,7 @@
return null;
}
}
-};
-/**
- * Get the bounding coordinates of the selection relative to the viewport.
- *
- * Returned coordinates are client coordinates (i.e. relative to the viewport).
- * For coordinates relative to the surface, use
#getSelectionBoundingRelativeRect
- *
- * @method
- * @returns {Object|null} Selection rectangle, with keys top, bottom, left,
right, width, height
- */
-ve.ce.Surface.prototype.getSelectionBoundingClientRect = function () {
- var inlineRects, boundingRect, surfaceRect, nativeRange;
-
- if ( this.focusedNode ) {
- boundingRect = this.focusedNode.getBoundingRect();
- surfaceRect = this.getSurface().getBoundingClientRect();
- if ( !boundingRect || !surfaceRect ) {
- return;
- }
- // Convert surface relative coords to client coords
- return ve.translateRect( boundingRect, surfaceRect.left,
surfaceRect.top );
- }
-
- // We can't do anything if there's no selection
- if ( this.nativeSelection.rangeCount === 0 ) {
- return null;
- }
-
- nativeRange = this.getNativeRange();
- if ( !nativeRange ) {
- return null;
- }
-
- try {
- inlineRects = nativeRange.getClientRects();
- // Try the zeroth inline rect first as Chrome sometimes returns
a rectangle
- // full of zeros for getBoundingClientRect when the cursor is
collapsed.
- // We could test for this failure and fall back to inline[0],
except for the
- // fact that the bounding rect is 1px bigger than inline[0], so
cursoring across
- // a link causes a verticle wobble as it alternately breaks and
unbreaks.
- // See
https://code.google.com/p/chromium/issues/detail?id=238976
- if ( inlineRects.length === 1 ) {
- return inlineRects[0];
- }
- return nativeRange.getBoundingClientRect();
- } catch ( e ) {
- return null;
- }
-};
-
-/**
- * Get the inline coordinates of the selection range relative to the surface.
- *
- * Returned coordinates are relative to the surface. For client coordinates,
- * use #getSelectionInlineClientRects.
- *
- * @method
- * @returns {Object|null} Start and end selection rectangles
- */
-ve.ce.Surface.prototype.getSelectionInlineRelativeRects = function () {
- var inlineRects, surfaceRect;
-
- if ( this.focusedNode ) {
- // We can optimize the focusedNode case as we already have the
relative coordinates
- return this.focusedNode.getInlineRects();
- }
-
- inlineRects = this.getSelectionInlineClientRects();
surfaceRect = this.getSurface().getBoundingClientRect();
if ( !inlineRects || !surfaceRect ) {
return null;
@@ -461,20 +384,23 @@
/**
* Get the coordinates of the selection anchor relative to the surface.
*
- * Returned coordinates are relative to the surface. For client coordinates,
- * use #getSelectionBoundingClientRect.
+ * Returned coordinates are relative to the surface.
*
* @method
* @returns {Object|null} Selection rectangle, with keys top, bottom, left,
right, width, height
*/
-ve.ce.Surface.prototype.getSelectionBoundingRelativeRect = function () {
- var boundingRect, surfaceRect;
+ve.ce.Surface.prototype.getSelectionBoundingRect = function () {
+ var nativeRange, boundingRect, surfaceRect;
if ( this.focusedNode ) {
- // We can optimize the focusedNode case as we already have the
relative coordinates
return this.focusedNode.getBoundingRect();
}
- boundingRect = this.getSelectionBoundingClientRect();
+ nativeRange = this.getNativeRange();
+ if ( !nativeRange ) {
+ return null;
+ }
+
+ boundingRect = this.getNativeRangeBoundingClientRect( nativeRange );
surfaceRect = this.getSurface().getBoundingClientRect();
if ( !boundingRect || !surfaceRect ) {
return null;
@@ -2467,6 +2393,44 @@
};
/**
+ * Get bounding client rect of a native range
+ *
+ * Works around some browser bugs in Range#getBoundingClientRect
+ *
+ * @param {Range} nativeRange Native range to get the bounding client rect of
+ * @return {ClientRect|null} Client rectangle of the native selection, or null
if there was a problem
+ */
+ve.ce.Surface.prototype.getNativeRangeBoundingClientRect = function (
nativeRange ) {
+ var inlineRects;
+
+ if ( !nativeRange ) {
+ return null;
+ }
+
+ try {
+ inlineRects = nativeRange.getClientRects();
+ if ( inlineRects.length === 0 ) {
+ // If there are no inline rects return null, otherwise
we'll fall through to
+ // getBoundingClientRect, which in Chrome becomes
[0,0,0,0].
+ return null;
+ } else if ( inlineRects.length === 1 ) {
+ // Try the zeroth inline rect first as Chrome sometimes
returns a rectangle
+ // full of zeros for getBoundingClientRect when the
cursor is collapsed.
+ // We could test for this failure and fall back to
inline[0], except for the
+ // fact that the bounding rect is 1px bigger than
inline[0], so cursoring across
+ // a link causes a verticle wobble as it alternately
breaks and unbreaks.
+ // See
https://code.google.com/p/chromium/issues/detail?id=238976
+ return inlineRects[0];
+ } else {
+ // After two browser bugs it's finally safe to try the
bounding rect.
+ return nativeRange.getBoundingClientRect();
+ }
+ } catch ( e ) {
+ return null;
+ }
+};
+
+/**
* Append passed highlights to highlight container.
*
* @method
diff --git a/src/ui/ve.ui.DesktopContext.js b/src/ui/ve.ui.DesktopContext.js
index 06ec337..9787bba 100644
--- a/src/ui/ve.ui.DesktopContext.js
+++ b/src/ui/ve.ui.DesktopContext.js
@@ -182,7 +182,7 @@
rtl = this.surface.getModel().getDocument().getDir() === 'rtl',
surface = this.surface.getView(),
focusedNode = surface.getFocusedNode(),
- boundingRect = surface.getSelectionBoundingRelativeRect();
+ boundingRect = surface.getSelectionBoundingRect();
$container = this.inspector ? this.inspector.$frame :
this.menu.$element;
@@ -209,7 +209,7 @@
}
} else {
// The selection is text or an inline focused node
- inlineRects = surface.getSelectionInlineRelativeRects();
+ inlineRects = surface.getSelectionInlineRects();
if ( inlineRects && boundingRect ) {
middle = ( boundingRect.left + boundingRect.right ) / 2;
if (
@@ -229,7 +229,7 @@
};
}
}
- // If !inlineRects, the surface apparently isn't selected, so
getSelectionBoundingRelativeRect()
+ // If !inlineRects, the surface apparently isn't selected, so
getSelectionBoundingRect()
// returned null. This shouldn't happen because the context is
only supposed to be
// displayed in response to a selection, but for some reason
this does happen when opening
// an inspector without changing the selection.
diff --git a/src/ui/ve.ui.Toolbar.js b/src/ui/ve.ui.Toolbar.js
index 04f2616..a0e44c0 100644
--- a/src/ui/ve.ui.Toolbar.js
+++ b/src/ui/ve.ui.Toolbar.js
@@ -140,13 +140,20 @@
* FIXME: this code should be in a target class or something
*/
ve.ui.Toolbar.prototype.onSurfaceViewKeyUp = function () {
- var clientRect, barHeight, scrollTo, obscured;
+ var surfaceView, nativeRange, clientRect, barHeight, scrollTo, obscured;
if ( !this.floating ) {
return;
}
- clientRect =
this.getSurface().getView().getSelectionBoundingClientRect();
+ surfaceView = this.getSurface().getView();
+
+ nativeRange = surfaceView.getNativeRange();
+ if ( !nativeRange ) {
+ return null;
+ }
+
+ clientRect = surfaceView.getNativeRangeBoundingClientRect( nativeRange
);
if ( !clientRect ) {
return;
}
diff --git a/tests/ce/ve.ce.Surface.test.js b/tests/ce/ve.ce.Surface.test.js
index 4cd7aa7..c0ee222 100644
--- a/tests/ce/ve.ce.Surface.test.js
+++ b/tests/ce/ve.ce.Surface.test.js
@@ -1088,10 +1088,8 @@
// TODO: ve.ce.Surface#getDocument
// TODO: ve.ce.Surface#getFocusedNode
// TODO: ve.ce.Surface#isRenderingLocked
-// TODO: ve.ce.Surface#getSelectionBoundingClientRect
-// TODO: ve.ce.Surface#getSelectionBoundingRelativeRect
-// TODO: ve.ce.Surface#getSelectionInlineClientRects
-// TODO: ve.ce.Surface#getSelectionInlineRelativeRects
+// TODO: ve.ce.Surface#getSelectionBoundingRect
+// TODO: ve.ce.Surface#getSelectionInlineRects
/* Methods without return values */
// TODO: ve.ce.Surface#initialize
--
To view, visit https://gerrit.wikimedia.org/r/160579
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I92fdcc24c6114e5e02d5bc5b227b1660d2429e52
Gerrit-PatchSet: 1
Gerrit-Project: VisualEditor/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Esanders <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
