зеркало из https://github.com/mozilla/gecko-dev.git
Bug 1533293 - part 1: Create Selection::SetStartAndEnd() to set new range as far as faster r=smaug
`Selection::Extend()` is too slow because: - it may create some `nsRange` instances. - it users `nsContentUtils::ComparePoints()` multiple times. Therefore, we can improve the performance if we can stop using it in some places. First, this patch creates `Selection::SetStartAndEnd()` and `Selection::SetStartAndEndInLimiter()` for internal use. They remove current ranges, reuse `nsRange` instance as far as possible and add new range which is set by their arguments. Then, this patch makes `Selection::SelectAllChildren()` stop using `Selection::Extend()`. At this time, this fixes a web-compat issue. `Selection::Expand()` cannot cross the selection limiter boundary when there is a limiter (e.g., when an editing host has focus). But we can now fix this with using the new internal API. Note that methods in editor shouldn't move selection to outside of active editing host. Therefore, this patch adds `Selection::SetStartAndEndInLimiter()` and `Selection::SetBaseAndExtentInLimiter()` for them. Differential Revision: https://phabricator.services.mozilla.com/D23459 --HG-- extra : moz-landing-system : lando
This commit is contained in:
Masayuki Nakano
Родитель
cbbed3707e
Коммит
d4cbc09db8
|
|
@ -2763,14 +2763,12 @@ void Selection::SelectAllChildren(nsINode& aNode, ErrorResult& aRv) {
|
|||
if (mFrameSelection) {
|
||||
mFrameSelection->PostReason(nsISelectionListener::SELECTALL_REASON);
|
||||
}
|
||||
SelectionBatcher batch(this);
|
||||
|
||||
Collapse(aNode, 0, aRv);
|
||||
if (aRv.Failed()) {
|
||||
return;
|
||||
}
|
||||
|
||||
Extend(aNode, aNode.GetChildCount(), aRv);
|
||||
// Chrome moves focus when aNode is outside of active editing host.
|
||||
// So, we don't need to respect the limiter with this method.
|
||||
SetStartAndEndInternal(InLimiter::eNo, RawRangeBoundary(&aNode, 0),
|
||||
RawRangeBoundary(&aNode, aNode.GetChildCount()),
|
||||
eDirNext, aRv);
|
||||
}
|
||||
|
||||
bool Selection::ContainsNode(nsINode& aNode, bool aAllowPartial,
|
||||
|
|
@ -3390,43 +3388,86 @@ void Selection::SetBaseAndExtentJS(nsINode& aAnchorNode, uint32_t aAnchorOffset,
|
|||
SetBaseAndExtent(aAnchorNode, aAnchorOffset, aFocusNode, aFocusOffset, aRv);
|
||||
}
|
||||
|
||||
void Selection::SetBaseAndExtent(nsINode& aAnchorNode, uint32_t aAnchorOffset,
|
||||
nsINode& aFocusNode, uint32_t aFocusOffset,
|
||||
ErrorResult& aRv) {
|
||||
void Selection::SetBaseAndExtentInternal(InLimiter aInLimiter,
|
||||
const RawRangeBoundary& aAnchorRef,
|
||||
const RawRangeBoundary& aFocusRef,
|
||||
ErrorResult& aRv) {
|
||||
if (!mFrameSelection) {
|
||||
return;
|
||||
}
|
||||
|
||||
if (!HasSameRoot(aAnchorNode) || !HasSameRoot(aFocusNode)) {
|
||||
if (NS_WARN_IF(!aAnchorRef.IsSet()) || NS_WARN_IF(!aFocusRef.IsSet())) {
|
||||
aRv.Throw(NS_ERROR_INVALID_ARG);
|
||||
return;
|
||||
}
|
||||
|
||||
if (!HasSameRoot(*aAnchorRef.Container()) ||
|
||||
!HasSameRoot(*aFocusRef.Container())) {
|
||||
// Return with no error
|
||||
return;
|
||||
}
|
||||
|
||||
// Prevent "selectionchange" event temporarily because it should be fired
|
||||
// after we set the direction.
|
||||
// XXX If they are disconnected, shouldn't we return error before allocating
|
||||
// new nsRange instance?
|
||||
SelectionBatcher batch(this);
|
||||
if (nsContentUtils::ComparePoints(aAnchorRef, aFocusRef) <= 0) {
|
||||
SetStartAndEndInternal(aInLimiter, aAnchorRef, aFocusRef, eDirNext, aRv);
|
||||
return;
|
||||
}
|
||||
|
||||
SetStartAndEndInternal(aInLimiter, aFocusRef, aAnchorRef, eDirPrevious, aRv);
|
||||
}
|
||||
|
||||
void Selection::SetStartAndEndInternal(InLimiter aInLimiter,
|
||||
const RawRangeBoundary& aStartRef,
|
||||
const RawRangeBoundary& aEndRef,
|
||||
nsDirection aDirection,
|
||||
ErrorResult& aRv) {
|
||||
if (NS_WARN_IF(!aStartRef.IsSet()) || NS_WARN_IF(!aEndRef.IsSet())) {
|
||||
aRv.Throw(NS_ERROR_INVALID_ARG);
|
||||
return;
|
||||
}
|
||||
|
||||
// Don't fire "selectionchange" event until everything done.
|
||||
SelectionBatcher batch(this);
|
||||
|
||||
int32_t relativePosition = nsContentUtils::ComparePoints(
|
||||
&aAnchorNode, aAnchorOffset, &aFocusNode, aFocusOffset);
|
||||
nsINode* start = &aAnchorNode;
|
||||
nsINode* end = &aFocusNode;
|
||||
uint32_t startOffset = aAnchorOffset;
|
||||
uint32_t endOffset = aFocusOffset;
|
||||
if (relativePosition > 0) {
|
||||
start = &aFocusNode;
|
||||
end = &aAnchorNode;
|
||||
startOffset = aFocusOffset;
|
||||
endOffset = aAnchorOffset;
|
||||
if (aInLimiter == InLimiter::eYes) {
|
||||
if (!IsValidSelectionPoint(mFrameSelection, aStartRef.Container())) {
|
||||
aRv.Throw(NS_ERROR_FAILURE);
|
||||
return;
|
||||
}
|
||||
if (aStartRef.Container() != aEndRef.Container() &&
|
||||
!IsValidSelectionPoint(mFrameSelection, aEndRef.Container())) {
|
||||
aRv.Throw(NS_ERROR_FAILURE);
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
// If we're not called by JS, we can remove all ranges first. Then, we
|
||||
// may be able to reuse one of current ranges for reducing the cost of
|
||||
// nsRange allocation. Note that if this is called by
|
||||
// SetBaseAndExtentJS(), when we fail to initialize new range, we
|
||||
// shouldn't remove current ranges. Therefore, we need to check whether
|
||||
// we're called by JS or internally.
|
||||
if (!mCalledByJS && !mCachedRange) {
|
||||
nsresult rv = RemoveAllRangesTemporarily();
|
||||
if (NS_WARN_IF(NS_FAILED(rv))) {
|
||||
aRv.Throw(rv);
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
// If there is cached range, we should reuse it for saving the allocation
|
||||
// const (and some other cost in nsRange::DoSetRange().
|
||||
// const (and some other cost in nsRange::DoSetRange()).
|
||||
RefPtr<nsRange> newRange = std::move(mCachedRange);
|
||||
|
||||
nsresult rv = NS_OK;
|
||||
if (newRange) {
|
||||
rv = newRange->SetStartAndEnd(start, startOffset, end, endOffset);
|
||||
rv = newRange->SetStartAndEnd(aStartRef, aEndRef);
|
||||
} else {
|
||||
rv = nsRange::CreateRange(start, startOffset, end, endOffset,
|
||||
getter_AddRefs(newRange));
|
||||
rv = nsRange::CreateRange(aStartRef, aEndRef, getter_AddRefs(newRange));
|
||||
}
|
||||
|
||||
// nsRange::SetStartAndEnd() and nsRange::CreateRange() returns
|
||||
|
|
@ -3446,7 +3487,7 @@ void Selection::SetBaseAndExtent(nsINode& aAnchorNode, uint32_t aAnchorOffset,
|
|||
return;
|
||||
}
|
||||
|
||||
SetDirection(relativePosition > 0 ? eDirPrevious : eDirNext);
|
||||
SetDirection(aDirection);
|
||||
}
|
||||
|
||||
/** SelectionLanguageChange modifies the cursor Bidi level after a change in
|
||||
|
|
|
|||
|
|
@ -349,6 +349,7 @@ class Selection final : public nsSupportsWeakReference,
|
|||
void Modify(const nsAString& aAlter, const nsAString& aDirection,
|
||||
const nsAString& aGranularity, mozilla::ErrorResult& aRv);
|
||||
|
||||
MOZ_CAN_RUN_SCRIPT
|
||||
void SetBaseAndExtentJS(nsINode& aAnchorNode, uint32_t aAnchorOffset,
|
||||
nsINode& aFocusNode, uint32_t aFocusOffset,
|
||||
mozilla::ErrorResult& aRv);
|
||||
|
|
@ -444,9 +445,84 @@ class Selection final : public nsSupportsWeakReference,
|
|||
MOZ_CAN_RUN_SCRIPT_BOUNDARY
|
||||
void SelectAllChildren(nsINode& aNode, mozilla::ErrorResult& aRv);
|
||||
|
||||
/**
|
||||
* SetStartAndEnd() removes all ranges and sets new range as given range.
|
||||
* Different from SetBaseAndExtent(), this won't compare the DOM points of
|
||||
* aStartRef and aEndRef for performance nor set direction to eDirPrevious.
|
||||
* Note that this may reset the limiter and move focus. If you don't want
|
||||
* that, use SetStartAndEndInLimiter() instead.
|
||||
*/
|
||||
MOZ_CAN_RUN_SCRIPT
|
||||
void SetStartAndEnd(const RawRangeBoundary& aStartRef,
|
||||
const RawRangeBoundary& aEndRef, ErrorResult& aRv) {
|
||||
SetStartAndEndInternal(InLimiter::eNo, aStartRef, aEndRef, eDirNext, aRv);
|
||||
}
|
||||
MOZ_CAN_RUN_SCRIPT
|
||||
void SetStartAndEnd(nsINode& aStartContainer, uint32_t aStartOffset,
|
||||
nsINode& aEndContainer, uint32_t aEndOffset,
|
||||
ErrorResult& aRv) {
|
||||
SetStartAndEnd(RawRangeBoundary(&aStartContainer, aStartOffset),
|
||||
RawRangeBoundary(&aEndContainer, aEndOffset), aRv);
|
||||
}
|
||||
|
||||
/**
|
||||
* SetStartAndEndInLimiter() is similar to SetStartAndEnd(), but this respects
|
||||
* the selection limiter. If all or part of given range is not in the
|
||||
* limiter, this returns error.
|
||||
*/
|
||||
MOZ_CAN_RUN_SCRIPT
|
||||
void SetStartAndEndInLimiter(const RawRangeBoundary& aStartRef,
|
||||
const RawRangeBoundary& aEndRef,
|
||||
ErrorResult& aRv) {
|
||||
SetStartAndEndInternal(InLimiter::eYes, aStartRef, aEndRef, eDirNext, aRv);
|
||||
}
|
||||
MOZ_CAN_RUN_SCRIPT
|
||||
void SetStartAndEndInLimiter(nsINode& aStartContainer, uint32_t aStartOffset,
|
||||
nsINode& aEndContainer, uint32_t aEndOffset,
|
||||
ErrorResult& aRv) {
|
||||
SetStartAndEndInLimiter(RawRangeBoundary(&aStartContainer, aStartOffset),
|
||||
RawRangeBoundary(&aEndContainer, aEndOffset), aRv);
|
||||
}
|
||||
|
||||
/**
|
||||
* SetBaseAndExtent() is alternative of the JS API for internal use.
|
||||
* Different from SetStartAndEnd(), this sets anchor and focus points as
|
||||
* specified, then if anchor point is after focus node, this sets the
|
||||
* direction to eDirPrevious.
|
||||
* Note that this may reset the limiter and move focus. If you don't want
|
||||
* that, use SetBaseAndExtentInLimier() instead.
|
||||
*/
|
||||
MOZ_CAN_RUN_SCRIPT
|
||||
void SetBaseAndExtent(nsINode& aAnchorNode, uint32_t aAnchorOffset,
|
||||
nsINode& aFocusNode, uint32_t aFocusOffset,
|
||||
mozilla::ErrorResult& aRv);
|
||||
ErrorResult& aRv) {
|
||||
SetBaseAndExtent(RawRangeBoundary(&aAnchorNode, aAnchorOffset),
|
||||
RawRangeBoundary(&aFocusNode, aFocusOffset), aRv);
|
||||
}
|
||||
MOZ_CAN_RUN_SCRIPT
|
||||
void SetBaseAndExtent(const RawRangeBoundary& aAnchorRef,
|
||||
const RawRangeBoundary& aFocusRef, ErrorResult& aRv) {
|
||||
SetBaseAndExtentInternal(InLimiter::eNo, aAnchorRef, aFocusRef, aRv);
|
||||
}
|
||||
|
||||
/**
|
||||
* SetBaseAndExtentInLimier() is similar to SetBaseAndExtent(), but this
|
||||
* respects the selection limiter. If all or part of given range is not in
|
||||
* the limiter, this returns error.
|
||||
*/
|
||||
MOZ_CAN_RUN_SCRIPT
|
||||
void SetBaseAndExtentInLimiter(nsINode& aAnchorNode, uint32_t aAnchorOffset,
|
||||
nsINode& aFocusNode, uint32_t aFocusOffset,
|
||||
ErrorResult& aRv) {
|
||||
SetBaseAndExtentInLimiter(RawRangeBoundary(&aAnchorNode, aAnchorOffset),
|
||||
RawRangeBoundary(&aFocusNode, aFocusOffset), aRv);
|
||||
}
|
||||
MOZ_CAN_RUN_SCRIPT
|
||||
void SetBaseAndExtentInLimiter(const RawRangeBoundary& aAnchorRef,
|
||||
const RawRangeBoundary& aFocusRef,
|
||||
ErrorResult& aRv) {
|
||||
SetBaseAndExtentInternal(InLimiter::eYes, aAnchorRef, aFocusRef, aRv);
|
||||
}
|
||||
|
||||
void AddSelectionChangeBlocker();
|
||||
void RemoveSelectionChangeBlocker();
|
||||
|
|
@ -536,6 +612,25 @@ class Selection final : public nsSupportsWeakReference,
|
|||
nsresult GetCachedFrameOffset(nsIFrame* aFrame, int32_t inOffset,
|
||||
nsPoint& aPoint);
|
||||
|
||||
enum class InLimiter {
|
||||
// If eYes, the method may reset selection limiter and move focus if the
|
||||
// given range is out of the limiter.
|
||||
eYes,
|
||||
// If eNo, the method won't reset selection limiter. So, if given range
|
||||
// is out of bounds, the method may return error.
|
||||
eNo,
|
||||
};
|
||||
MOZ_CAN_RUN_SCRIPT
|
||||
void SetStartAndEndInternal(InLimiter aInLimiter,
|
||||
const RawRangeBoundary& aStartRef,
|
||||
const RawRangeBoundary& aEndRef,
|
||||
nsDirection aDirection, ErrorResult& aRv);
|
||||
MOZ_CAN_RUN_SCRIPT
|
||||
void SetBaseAndExtentInternal(InLimiter aInLimiter,
|
||||
const RawRangeBoundary& aAnchorRef,
|
||||
const RawRangeBoundary& aFocusRef,
|
||||
ErrorResult& aRv);
|
||||
|
||||
public:
|
||||
SelectionType GetType() const { return mSelectionType; }
|
||||
void SetType(SelectionType aSelectionType) {
|
||||
|
|
|
|||
|
|
@ -38,9 +38,7 @@ function test() {
|
|||
synthesizeKey("KEY_Enter");
|
||||
synthesizeKey("KEY_Backspace");
|
||||
synthesizeKey("KEY_Backspace");
|
||||
// For some reason this test fails if the separator is not "br"
|
||||
}, () => document.queryCommandValue("defaultParagraphSeparator") == "br"
|
||||
? undefined : " A; B ; C "],
|
||||
}],
|
||||
["adding shift-returns", () => {
|
||||
getSelection().collapse(edit.firstChild, 0);
|
||||
synthesizeKey("KEY_ArrowRight");
|
||||
|
|
|
|||
|
|
@ -1,41 +0,0 @@
|
|||
[Selection_selectAllChildren.html]
|
||||
type: testharness
|
||||
[Active element should be 'editor' after Selection.selectAllChildren() to select the children of 'staticBefore' when active element is 'editor']
|
||||
expected: FAIL
|
||||
|
||||
[Active element should be 'outerEditor' after Selection.selectAllChildren() to select the children of 'outerEditor' when active element is 'editor']
|
||||
expected: FAIL
|
||||
|
||||
[Active element should be 'editor' after Selection.selectAllChildren() to select the children of 'staticInEditor' when active element is 'editor']
|
||||
expected: FAIL
|
||||
|
||||
[Active element should be 'innerEditor' after Selection.selectAllChildren() to select the children of 'innerEditor' when active element is 'editor']
|
||||
expected: FAIL
|
||||
|
||||
[Active element should be 'editor' after Selection.selectAllChildren() to select the children of 'anchor' when active element is 'editor']
|
||||
expected: FAIL
|
||||
|
||||
[Active element should be 'outerEditor' after Selection.selectAllChildren() to select the children of 'staticBefore' when active element is 'outerEditor']
|
||||
expected: FAIL
|
||||
|
||||
[Active element should be 'editor' after Selection.selectAllChildren() to select the children of 'editor' when active element is 'outerEditor']
|
||||
expected: FAIL
|
||||
|
||||
[Active element should be 'outerEditor' after Selection.selectAllChildren() to select the children of 'anchor' when active element is 'outerEditor']
|
||||
expected: FAIL
|
||||
|
||||
[Active element should be 'innerEditor' after Selection.selectAllChildren() to select the children of 'staticBefore' when active element is 'innerEditor']
|
||||
expected: FAIL
|
||||
|
||||
[Active element should be 'editor' after Selection.selectAllChildren() to select the children of 'editor' when active element is 'innerEditor']
|
||||
expected: FAIL
|
||||
|
||||
[Active element should be 'outerEditor' after Selection.selectAllChildren() to select the children of 'outerEditor' when active element is 'innerEditor']
|
||||
expected: FAIL
|
||||
|
||||
[Active element should be 'innerEditor' after Selection.selectAllChildren() to select the children of 'staticInEditor' when active element is 'innerEditor']
|
||||
expected: FAIL
|
||||
|
||||
[Active element should be 'innerEditor' after Selection.selectAllChildren() to select the children of 'anchor' when active element is 'innerEditor']
|
||||
expected: FAIL
|
||||
|
||||
Загрузка…
Ссылка в новой задаче