Closed Bug 1424676 Opened 2 years ago Closed 2 years ago

TextEditor::CreateBR() should take EditorRawDOMPoint

Categories

(Core :: DOM: Editor, enhancement)

56 Branch
enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox59 --- fixed

People

(Reporter: masayuki, Assigned: masayuki)

Details

Attachments

(3 files)

No description provided.
Comment on attachment 8936318 [details]
Bug 1424676 - part 1: Make TextEditor::CreateBR() take |const EditorRawDOMPoint&| to specify the insertion point

https://reviewboard.mozilla.org/r/207040/#review212928

::: editor/libeditor/HTMLEditRules.cpp:3418
(Diff revision 1)
> -      NS_ENSURE_SUCCESS(rv, rv);
> -      if (parent && seenBR) {
> -        NS_ENSURE_STATE(mHTMLEditor);
> -        nsCOMPtr<Element> brNode = mHTMLEditor->CreateBR(parent, offset);
> -        NS_ENSURE_STATE(brNode);
> -        aSelection->Collapse(parent, offset);
> +        if (NS_WARN_IF(NS_FAILED(rv))) {
> +          return rv;
> +        }
> +      }
> +      if (atCiteNode.IsSet() && seenBR) {
> +        RefPtr<Element> brNode = htmlEditor->CreateBR(atCiteNode.AsRaw());

I think that it can use htmlEditor->CreateBR(atCiteNode.AsRow(), ePrevious) and remove Collapse in this block.  But it should handle another bug. (There is some code for this situation)

::: editor/libeditor/HTMLEditor.cpp:3736
(Diff revision 1)
>  
>      nsCOMPtr<nsIContent> sibling = GetPriorHTMLSibling(&aNode);
>      if (sibling && !IsBlockNode(sibling) &&
>          !sibling->IsHTMLElement(nsGkAtoms::br) && !IsBlockNode(child)) {
>        // Insert br node
> -      nsCOMPtr<Element> br = CreateBR(&aNode, 0);
> +      nsCOMPtr<Element> br = CreateBR(EditorRawDOMPoint(&aNode, 0));

RefPtr instead of nsCOMPtr

::: editor/libeditor/HTMLEditor.cpp:3754
(Diff revision 1)
>        MOZ_ASSERT(child, "aNode has first editable child but not last?");
>        if (!IsBlockNode(child) && !child->IsHTMLElement(nsGkAtoms::br)) {
>          // Insert br node
> -        nsCOMPtr<Element> br = CreateBR(&aNode, aNode.Length());
> +        EditorRawDOMPoint endOfNode;
> +        endOfNode.SetToEndOf(&aNode);
> +        nsCOMPtr<Element> br = CreateBR(endOfNode);

RefPtr instead of nsCOMPtr

::: editor/libeditor/HTMLEditor.cpp:3772
(Diff revision 1)
>          !sibling->IsHTMLElement(nsGkAtoms::br)) {
>        sibling = GetNextHTMLSibling(&aNode);
>        if (sibling && !IsBlockNode(sibling) &&
>            !sibling->IsHTMLElement(nsGkAtoms::br)) {
>          // Insert br node
> -        nsCOMPtr<Element> br = CreateBR(&aNode, 0);
> +        nsCOMPtr<Element> br = CreateBR(EditorRawDOMPoint(&aNode, 0));

RefPtr instead of nsCOMPtr
Attachment #8936318 - Flags: review?(m_kato) → review+
Comment on attachment 8936319 [details]
Bug 1424676 - part 2: Redesign TextEditRules::CreateBR(), TextEditRules::CreateMozBR() and TextEditRules::CreateBRInternal() with |const EditorRawDOMPoint&|

https://reviewboard.mozilla.org/r/207042/#review212930

::: editor/libeditor/HTMLEditRules.cpp:8082
(Diff revision 1)
>      }
>    }
>  
>    // make sure we aren't in an empty block - user will see no cursor.  If this
>    // is happening, put a <br> in the block if allowed.
> -  NS_ENSURE_STATE(mHTMLEditor);
> +  nsCOMPtr<Element> theblock = htmlEditor->GetBlock(*point.GetContainer());

RefPtr instead of nsCOMPtr

::: editor/libeditor/HTMLEditRules.cpp:8092
(Diff revision 1)
>      NS_ENSURE_SUCCESS(rv, rv);
>      // check if br can go into the destination node
> -    NS_ENSURE_STATE(mHTMLEditor);
>      if (bIsEmptyNode &&
> -        mHTMLEditor->CanContainTag(*point.GetContainer(), *nsGkAtoms::br)) {
> -      NS_ENSURE_STATE(mHTMLEditor);
> +        htmlEditor->CanContainTag(*point.GetContainer(), *nsGkAtoms::br)) {
> +      nsCOMPtr<Element> rootNode = htmlEditor->GetRoot();

Is refcounting requried?  I think that the following is better.
```
Element* rootNoe = htmlEditor->GetRoot();
```

::: editor/libeditor/HTMLEditRules.cpp:8124
(Diff revision 1)
> -  NS_ENSURE_STATE(mHTMLEditor);
>    nsCOMPtr<nsIContent> nearNode =
> -    mHTMLEditor->GetPreviousEditableHTMLNode(point.AsRaw());
> +    htmlEditor->GetPreviousEditableHTMLNode(point.AsRaw());
>    if (nearNode) {
>      // is nearNode also a descendant of same block?
> -    NS_ENSURE_STATE(mHTMLEditor);
> +    nsCOMPtr<Element> block = htmlEditor->GetBlock(*point.GetContainer());

Use RefPtr instead of nsCOMPtr.

::: editor/libeditor/HTMLEditRules.cpp:8125
(Diff revision 1)
>    nsCOMPtr<nsIContent> nearNode =
> -    mHTMLEditor->GetPreviousEditableHTMLNode(point.AsRaw());
> +    htmlEditor->GetPreviousEditableHTMLNode(point.AsRaw());
>    if (nearNode) {
>      // is nearNode also a descendant of same block?
> -    NS_ENSURE_STATE(mHTMLEditor);
> -    nsCOMPtr<Element> block = mHTMLEditor->GetBlock(*point.GetContainer());
> +    nsCOMPtr<Element> block = htmlEditor->GetBlock(*point.GetContainer());
> +    nsCOMPtr<Element> nearBlock = htmlEditor->GetBlockNodeParent(nearNode);

Use RefPtr instead of nsCOMPtr.
Attachment #8936319 - Flags: review?(m_kato) → review+
Comment on attachment 8936320 [details]
Bug 1424676 - part 3: Redesign HTMLEditRules::StandardBreakImpl() with |const EditorDOMPoint&|

https://reviewboard.mozilla.org/r/207044/#review212932

::: editor/libeditor/HTMLEditRules.cpp:1884
(Diff revision 1)
>      }
> +    // If the container of the break is a link, we need to split it and
> +    // insert new <br> between the split links.
>      nsCOMPtr<nsIDOMNode> linkDOMNode;
> -    if (htmlEditor->IsInLink(GetAsDOMNode(node), address_of(linkDOMNode))) {
> -      // Split the link
> +    if (htmlEditor->IsInLink(pointToBreak.GetContainerAsDOMNode(),
> +                             address_of(linkDOMNode))) {

I should add Element version of IsInLink
Attachment #8936320 - Flags: review?(m_kato) → review+
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/23ee038862c0
part 1: Make TextEditor::CreateBR() take |const EditorRawDOMPoint&| to specify the insertion point r=m_kato
https://hg.mozilla.org/integration/autoland/rev/67385db77eaf
part 2: Redesign TextEditRules::CreateBR(), TextEditRules::CreateMozBR() and TextEditRules::CreateBRInternal() with |const EditorRawDOMPoint&| r=m_kato
https://hg.mozilla.org/integration/autoland/rev/ffb14ef93c51
part 3: Redesign HTMLEditRules::StandardBreakImpl() with |const EditorDOMPoint&| r=m_kato
https://hg.mozilla.org/mozilla-central/rev/23ee038862c0
https://hg.mozilla.org/mozilla-central/rev/67385db77eaf
https://hg.mozilla.org/mozilla-central/rev/ffb14ef93c51
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
You need to log in before you can comment on or make changes to this bug.