1387945 - Use nsIContent for CreateAnonymousElement

Status: RESOLVED FIXED

(Core :: DOM: Editor, enhancement, P3)

Description

[Makoto Kato [:m_kato]]

CreateAnonymousElement still use nsIDOMNode, but we should use nsIContent for it.

Comment 1

[Makoto Kato [:m_kato]]

Attachment: Bug 1387945 - Use nsIContent for CreateAnonymousElement.

CreateAnonymousElement still uses nsIDOMNode, but we should use nsIContent for it.

Review commit: https://reviewboard.mozilla.org/r/165582/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/165582/

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 8894398 [details]
Bug 1387945 - Use nsIContent for CreateAnonymousElement.

https://reviewboard.mozilla.org/r/165582/#review170648

::: editor/libeditor/HTMLAbsPositionEditor.cpp:335
(Diff revision 1)
> -  nsCOMPtr<nsINode> parentNode = mGrabber->GetParentNode();
> +  nsCOMPtr<nsIContent> parentContent = mGrabber->GetParent();
>  
>    // now, let's create the resizing shadow
> -  mPositioningShadow = CreateShadow(GetAsDOMNode(parentNode),
> -      static_cast<nsIDOMElement*>(GetAsDOMNode(mAbsolutelyPositionedObject)));
> +  mPositioningShadow =
> +    CreateShadow(*parentContent, *mAbsolutelyPositionedObject);

Please check if parentContent is nullptr and mAbsolutelyPositionedObject is nullptr before that.

::: editor/libeditor/HTMLEditor.cpp:4811
(Diff revision 1)
>      return NS_ERROR_NOT_INITIALIZED;
>    }
>    nsCOMPtr<nsIPresShell> ps = GetPresShell();
>    NS_ENSURE_TRUE(ps, NS_ERROR_NOT_INITIALIZED);
>  
> -  nsCOMPtr<nsIContent> content = do_QueryInterface(aElement);
> +  nsIFrame *frame = aElement.GetPrimaryFrame();

|nsIFrame* frame| (position of *).

::: editor/libeditor/HTMLEditorObjectResizer.cpp:380
(Diff revision 1)
>    // XXX Even when it failed to add event listener, should we need to set
>    //     _moz_resizing attribute?
> -  aResizedElement->SetAttribute(NS_LITERAL_STRING("_moz_resizing"), NS_LITERAL_STRING("true"));
> -  return rv;
> +  aResizedElement.SetAttr(kNameSpaceID_None, nsGkAtoms::_moz_resizing,
> +                          NS_LITERAL_STRING("true"), true);
> +
> +  mResizedObject = &aResizedElement;

I think that this is wrong timing because this *might* be nested by mutation observers. So, the callers should be able to check if mResizedObject is already non-nullptr for hiding the creating anonymous elements.

So, please do this before creating mTopLeftHandle.

(Old code did so.)


On the other hand, after creating each element or set each attribute, this should check if mResizedObject equals aResizedElement.  However, this is not the scope of this bug.

Comment 3

[Makoto Kato [:m_kato]]

Comment on attachment 8894398 [details]
Bug 1387945 - Use nsIContent for CreateAnonymousElement.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/165582/diff/1-2/

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 8894398 [details]
Bug 1387945 - Use nsIContent for CreateAnonymousElement.

https://reviewboard.mozilla.org/r/165582/#review170662

::: editor/libeditor/HTMLEditorObjectResizer.cpp:382
(Diff revisions 1 - 2)
> -  mResizedObject = &aResizedElement;
> +  if (NS_WARN_IF(mResizedObject != &aResizedElement)) {
> +    return NS_ERROR_FAILURE;
> +  }

Perhaps, MOZ_ASSERT is enough, but up to you.

Comment 5

[Makoto Kato [:m_kato]]

Comment on attachment 8894398 [details]
Bug 1387945 - Use nsIContent for CreateAnonymousElement.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/165582/diff/2-3/

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e85bed50a2ac
Use nsIContent for CreateAnonymousElement. r=masayuki

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e85bed50a2ac