Bug 1484136 - Create HTMLEditor::RefreshGrabberInternal() for internal use r=m_kato

HTMLEditor::RefereshGrabber() is an XPCOM method which is used by BlueGriffon.
Additionally, it's called internally.  Therefore, we should create a non-virtual
method for this and all internal users should use it.

This patch renames all other related methods to *Internal() for consistency.
Additionally, this fixes a bug of nested calls of ShowGrabber() and
HideGrabber().  This makes CreateGrabber() sets mGrabber directly since
it may be cleared by HideGrabber() while it's running, and also makes
HideGrabber() moves all members who will be cleaned up with local variables
and always clean them up even if it meats an error.

Differential Revision: https://phabricator.services.mozilla.com/D5424

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Masayuki Nakano 2018-09-11 05:30:33 +00:00
Родитель 7108f5c2b6
Коммит a527ce6536
4 изменённых файлов: 112 добавлений и 61 удалений

Просмотреть файл

@ -220,85 +220,111 @@ HTMLEditor::GetZIndex(Element& aElement)
return zIndexStr.ToInteger(&errorCode);
}
ManualNACPtr
HTMLEditor::CreateGrabber(nsIContent& aParentContent)
bool
HTMLEditor::CreateGrabberInternal(nsIContent& aParentContent)
{
// let's create a grabber through the element factory
ManualNACPtr grabber =
CreateAnonymousElement(nsGkAtoms::span, aParentContent,
NS_LITERAL_STRING("mozGrabber"), false);
if (NS_WARN_IF(!grabber)) {
return nullptr;
if (NS_WARN_IF(mGrabber)) {
return false;
}
mGrabber = CreateAnonymousElement(nsGkAtoms::span, aParentContent,
NS_LITERAL_STRING("mozGrabber"), false);
// mGrabber may be destroyed during creation due to there may be
// mutation event listener.
if (NS_WARN_IF(!mGrabber)) {
return false;
}
// add the mouse listener so we can detect a click on a resizer
EventListenerManager* eventListenerManager =
grabber->GetOrCreateListenerManager();
mGrabber->GetOrCreateListenerManager();
eventListenerManager->AddEventListenerByType(
mEventListener,
NS_LITERAL_STRING("mousedown"),
TrustedEventsAtSystemGroupBubble());
return grabber;
MOZ_ASSERT(mGrabber);
return true;
}
NS_IMETHODIMP
HTMLEditor::RefreshGrabber()
{
NS_ENSURE_TRUE(mAbsolutelyPositionedObject, NS_ERROR_NULL_POINTER);
if (NS_WARN_IF(!mAbsolutelyPositionedObject)) {
return NS_ERROR_FAILURE;
}
nsresult rv =
GetPositionAndDimensions(
*mAbsolutelyPositionedObject,
mPositionedObjectX,
mPositionedObjectY,
mPositionedObjectWidth,
mPositionedObjectHeight,
mPositionedObjectBorderLeft,
mPositionedObjectBorderTop,
mPositionedObjectMarginLeft,
mPositionedObjectMarginTop);
NS_ENSURE_SUCCESS(rv, rv);
nsresult rv = RefreshGrabberInternal();
if (NS_WARN_IF(NS_FAILED(rv))) {
return rv;
}
return NS_OK;
}
SetAnonymousElementPosition(mPositionedObjectX+12,
mPositionedObjectY-14,
nsresult
HTMLEditor::RefreshGrabberInternal()
{
if (!mAbsolutelyPositionedObject) {
return NS_OK;
}
nsresult rv = GetPositionAndDimensions(*mAbsolutelyPositionedObject,
mPositionedObjectX,
mPositionedObjectY,
mPositionedObjectWidth,
mPositionedObjectHeight,
mPositionedObjectBorderLeft,
mPositionedObjectBorderTop,
mPositionedObjectMarginLeft,
mPositionedObjectMarginTop);
if (NS_WARN_IF(NS_FAILED(rv))) {
return rv;
}
SetAnonymousElementPosition(mPositionedObjectX + 12,
mPositionedObjectY - 14,
mGrabber);
return NS_OK;
}
void
HTMLEditor::HideGrabber()
HTMLEditor::HideGrabberInternal()
{
nsresult rv = mAbsolutelyPositionedObject->UnsetAttr(kNameSpaceID_None,
nsGkAtoms::_moz_abspos,
true);
if (NS_WARN_IF(NS_FAILED(rv))) {
if (NS_WARN_IF(!mAbsolutelyPositionedObject)) {
return;
}
mAbsolutelyPositionedObject = nullptr;
if (NS_WARN_IF(!mGrabber)) {
return;
}
// Move all members to the local variables first since mutation event
// listener may try to show grabber while we're hiding them.
RefPtr<Element> absolutePositioningObject =
std::move(mAbsolutelyPositionedObject);
ManualNACPtr grabber = std::move(mGrabber);
ManualNACPtr positioningShadow = std::move(mPositioningShadow);
DebugOnly<nsresult> rv =
absolutePositioningObject->UnsetAttr(kNameSpaceID_None,
nsGkAtoms::_moz_abspos,
true);
NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), "Failed to unset the attribute");
// get the presshell's document observer interface.
nsCOMPtr<nsIPresShell> ps = GetPresShell();
// We allow the pres shell to be null; when it is, we presume there
// are no document observers to notify, but we still want to
// UnbindFromTree.
DeleteRefToAnonymousNode(std::move(mGrabber), ps);
DeleteRefToAnonymousNode(std::move(mPositioningShadow), ps);
nsCOMPtr<nsIPresShell> presShell = GetPresShell();
if (grabber) {
DeleteRefToAnonymousNode(std::move(grabber), presShell);
}
if (positioningShadow) {
DeleteRefToAnonymousNode(std::move(positioningShadow), presShell);
}
}
nsresult
HTMLEditor::ShowGrabber(Element& aElement)
HTMLEditor::ShowGrabberInternal(Element& aElement)
{
if (NS_WARN_IF(!IsDescendantOfEditorRoot(&aElement))) {
return NS_ERROR_UNEXPECTED;
}
if (mGrabber) {
NS_ERROR("call HideGrabber first");
if (NS_WARN_IF(mGrabber)) {
return NS_ERROR_UNEXPECTED;
}
@ -311,7 +337,6 @@ HTMLEditor::ShowGrabber(Element& aElement)
classValue, true);
NS_ENSURE_SUCCESS(rv, rv);
// first, let's keep track of that element...
mAbsolutelyPositionedObject = &aElement;
nsIContent* parentContent = aElement.GetParent();
@ -319,13 +344,22 @@ HTMLEditor::ShowGrabber(Element& aElement)
return NS_ERROR_FAILURE;
}
mGrabber = CreateGrabber(*parentContent);
NS_ENSURE_TRUE(mGrabber, NS_ERROR_FAILURE);
if (NS_WARN_IF(!CreateGrabberInternal(*parentContent))) {
return NS_ERROR_FAILURE;
}
// If we succeeded to create the grabber, HideGrabberInternal() hasn't been
// called yet. So, mAbsolutelyPositionedObject should be non-nullptr.
MOZ_ASSERT(mAbsolutelyPositionedObject);
mHasShownGrabber = true;
// and set its position
return RefreshGrabber();
// Finally, move the grabber to proper position.
rv = RefreshGrabberInternal();
if (NS_WARN_IF(NS_FAILED(rv))) {
return rv;
}
return NS_OK;
}
nsresult
@ -446,8 +480,11 @@ HTMLEditor::SetFinalPosition(int32_t aX,
// we want one transaction only from a user's point of view
AutoPlaceholderBatch batchIt(this);
nsCOMPtr<Element> absolutelyPositionedObject = mAbsolutelyPositionedObject;
NS_ENSURE_STATE(absolutelyPositionedObject);
if (NS_WARN_IF(!mAbsolutelyPositionedObject)) {
return NS_ERROR_FAILURE;
}
OwningNonNull<Element> absolutelyPositionedObject =
*mAbsolutelyPositionedObject;
mCSSEditUtils->SetCSSPropertyPixels(*absolutelyPositionedObject,
*nsGkAtoms::top, newY);
mCSSEditUtils->SetCSSPropertyPixels(*absolutelyPositionedObject,

Просмотреть файл

@ -286,7 +286,7 @@ void
HTMLEditor::HideAnonymousEditingUIs()
{
if (mAbsolutelyPositionedObject) {
HideGrabber();
HideGrabberInternal();
NS_ASSERTION(!mAbsolutelyPositionedObject, "HideGrabber failed");
}
if (mInlineEditedCell) {
@ -309,7 +309,7 @@ HTMLEditor::HideAnonymousEditingUIsIfUnnecessary()
if (!IsAbsolutePositionEditorEnabled() && mAbsolutelyPositionedObject) {
// XXX If we're moving something, we need to cancel or commit the
// operation now.
HideGrabber();
HideGrabberInternal();
NS_ASSERTION(!mAbsolutelyPositionedObject, "HideGrabber failed");
}
if (!IsInlineTableEditorEnabled() && mInlineEditedCell) {
@ -420,7 +420,7 @@ HTMLEditor::RefereshEditingUI(Selection& aSelection)
if (IsAbsolutePositionEditorEnabled() && mAbsolutelyPositionedObject &&
absPosElement != mAbsolutelyPositionedObject) {
HideGrabber();
HideGrabberInternal();
NS_ASSERTION(!mAbsolutelyPositionedObject, "HideGrabber failed");
}
@ -468,12 +468,12 @@ HTMLEditor::RefereshEditingUI(Selection& aSelection)
if (IsAbsolutePositionEditorEnabled() && absPosElement &&
IsModifiableNode(*absPosElement) && absPosElement != hostContent) {
if (mAbsolutelyPositionedObject) {
nsresult rv = RefreshGrabber();
nsresult rv = RefreshGrabberInternal();
if (NS_WARN_IF(NS_FAILED(rv))) {
return rv;
}
} else {
nsresult rv = ShowGrabber(*absPosElement);
nsresult rv = ShowGrabberInternal(*absPosElement);
if (NS_WARN_IF(NS_FAILED(rv))) {
return rv;
}

Просмотреть файл

@ -1871,14 +1871,29 @@ protected: // Shouldn't be used by friend classes
* document. See chrome://editor/content/images/grabber.gif
* @param aElement [IN] the element
*/
nsresult ShowGrabber(Element& aElement);
nsresult ShowGrabberInternal(Element& aElement);
/**
* Setting grabber to proper position for current mAbsolutelyPositionedObject.
* For example, while an element has grabber, the element may be resized
* or repositioned by script or something. Then, you need to reset grabber
* position with this.
*/
nsresult RefreshGrabberInternal();
/**
* hide the grabber if it shown.
*/
void HideGrabber();
void HideGrabberInternal();
/**
* CreateGrabberInternal() creates a grabber for moving aParentContent.
* This sets mGrabber to the new grabber. If this returns true, it's
* always non-nullptr. Otherwise, i.e., the grabber is hidden during
* creation, this returns false.
*/
bool CreateGrabberInternal(nsIContent& aParentContent);
ManualNACPtr CreateGrabber(nsIContent& aParentContent);
nsresult StartMoving();
nsresult SetFinalPosition(int32_t aX, int32_t aY);
void AddPositioningOffset(int32_t& aX, int32_t& aY);
@ -2051,7 +2066,7 @@ protected:
int32_t mPositionedObjectBorderLeft;
int32_t mPositionedObjectBorderTop;
nsCOMPtr<Element> mAbsolutelyPositionedObject;
RefPtr<Element> mAbsolutelyPositionedObject;
ManualNACPtr mGrabber;
ManualNACPtr mPositioningShadow;

Просмотреть файл

@ -33,8 +33,7 @@ interface nsIHTMLAbsPosEditor : nsISupports
/**
* refreshes the grabber if it shown, possibly updating its position or
* even hiding it.
* FYI: Current user in script is only BlueGriffon.
*/
void refreshGrabber();
};