1446533 - Get rid of nsIDOMCharacterData

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, enhancement, P2)

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: part 1. Stop using nsIDOMComment in serializers

MozReview-Commit-ID: IBVkQJOi6O7

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: part 2. Get rid of nsIDOMComment

MozReview-Commit-ID: GGXPQnlwuUM

Comment 3

[Boris Zbarsky [:bzbarsky]]

Patches for this will fix bug 826414 too.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: part 1. Rename nsGenericDOMDataNode to CharacterData

This is not renaming the files yet; that will be a separate changeset.

MozReview-Commit-ID: 5TxkEiQlaKF

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: part 2. Rename nsGenericDOMDataNode.{h,cpp} to CharacterData

The DOMMatrix.cpp changes are because it was sneaking in headers via another
unified file.

MozReview-Commit-ID: GPp9WOywI5D

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: part 3. Remove nsIDOMCharacterData::Get/SetData

MozReview-Commit-ID: 5YeaCPwvIJH

Comment 7

[Boris Zbarsky [:bzbarsky]]

Attachment: part 4. Remove nsIDOMCharacterData::SubstringData

MozReview-Commit-ID: DhiN6mCOb34

Comment 8

[Boris Zbarsky [:bzbarsky]]

Attachment: part 5. Remove nsIDOMCharacterData::AppendData

MozReview-Commit-ID: JP809oJeQiX

Comment 9

[Boris Zbarsky [:bzbarsky]]

Attachment: part 6. Remove nsIDOMCharacterData::GetLength

MozReview-Commit-ID: FCBGyqPfC4B

Comment 10

[Boris Zbarsky [:bzbarsky]]

Attachment: part 7. Remove nsIDOMCharacterData::InsertData

MozReview-Commit-ID: 48XZ2J9ewHP

Comment 11

[Boris Zbarsky [:bzbarsky]]

Attachment: part 8. Remove nsIDOMCharacterData::DeleteData

MozReview-Commit-ID: 7100YyU5jOG

Comment 12

[Boris Zbarsky [:bzbarsky]]

Attachment: part 9. Remove nsIDOMCharacterData::ReplaceData

MozReview-Commit-ID: Lei6xZ2rw2K

Comment 13

[Boris Zbarsky [:bzbarsky]]

Attachment: part 10. Remove remaining nsIDOMCharacterData uses in editor

MozReview-Commit-ID: CfZNQRiDs43

Comment 14

[Boris Zbarsky [:bzbarsky]]

Attachment: part 11. Remove remaining nsIDOMCharacterData uses in range code

MozReview-Commit-ID: 8YLea3SmQQU

Comment 15

[Boris Zbarsky [:bzbarsky]]

Attachment: part 12. Remove nsIDOMCharacterData

MozReview-Commit-ID: KXex3Rjcire

Comment 16

[Boris Zbarsky [:bzbarsky]]

Attachment: part 4. Remove nsIDOMCharacterData::SubstringData

FromContent will be renamed to FromNode in bug 1447098.

MozReview-Commit-ID: DhiN6mCOb34

Comment 17

[Nika Layzell [:nika] (ni? for response)]

Comment on attachment 8959908 [details] [diff] [review]
part 1.  Rename nsGenericDOMDataNode to CharacterData

Review of attachment 8959908 [details] [diff] [review]:
-----------------------------------------------------------------

I did not read through this entire thing - I'm just gonna rubberstamp it :p

Comment 18

[Nika Layzell [:nika] (ni? for response)]

Comment on attachment 8959909 [details] [diff] [review]
part 2.  Rename nsGenericDOMDataNode.{h,cpp} to CharacterData

Review of attachment 8959909 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/base/DOMMatrix.cpp
@@ +11,5 @@
>  #include "mozilla/dom/DOMPoint.h"
>  #include "mozilla/dom/DOMPointBinding.h"
>  #include "mozilla/dom/BindingDeclarations.h"
>  #include "mozilla/dom/ToJSValue.h"
> +#include "mozilla/RuleNodeCacheConditions.h"

Oh the joys of unified builds

Comment 19

[Nika Layzell [:nika] (ni? for response)]

Comment on attachment 8959912 [details] [diff] [review]
part 5.  Remove nsIDOMCharacterData::AppendData

Review of attachment 8959912 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/base/CharacterData.cpp
@@ +227,5 @@
> +  nsresult rv = SetTextInternal(mText.GetLength(), 0, aData.BeginReading(),
> +                                aData.Length(), true);
> +  if (NS_FAILED(rv)) {
> +    aRv.Throw(rv);
> +  }  

trailing whitespace.

Comment 20

[Nika Layzell [:nika] (ni? for response)]

Comment on attachment 8959913 [details] [diff] [review]
part 6.  Remove nsIDOMCharacterData::GetLength

Review of attachment 8959913 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/base/nsRange.cpp
@@ +2731,1 @@
>      {

I just love how consistent our C++ style is in gecko.

Comment 21

[Nika Layzell [:nika] (ni? for response)]

Comment on attachment 8959914 [details] [diff] [review]
part 7.  Remove nsIDOMCharacterData::InsertData

Review of attachment 8959914 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/base/CharacterData.cpp
@@ +219,3 @@
>  {
>    nsresult rv = SetTextInternal(mText.GetLength(), 0, aData.BeginReading(),
>                                  aData.Length(), true);

Should this method be made to forward to InsertData?

@@ +232,5 @@
> +  nsresult rv = SetTextInternal(aOffset, 0, aData.BeginReading(),
> +                                aData.Length(), true);
> +  if (NS_FAILED(rv)) {
> +    aRv.Throw(rv);
> +  }  

trailing whitespace.

Comment 22

[Nika Layzell [:nika] (ni? for response)]

Comment on attachment 8959915 [details] [diff] [review]
part 8.  Remove nsIDOMCharacterData::DeleteData

Review of attachment 8959915 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/base/CharacterData.cpp
@@ +241,5 @@
>  {
> +  nsresult rv = SetTextInternal(aOffset, aCount, nullptr, 0, true);
> +  if (NS_FAILED(rv)) {
> +    aRv.Throw(rv);
> +  }  

I have a feeling you copy-pasted the same function a bunch of times :p

::: editor/libeditor/CompositionTransaction.cpp
@@ +131,5 @@
>        while (node && node->IsNodeOfType(nsINode::eTEXT) &&
>               remainLength > 0) {
>          Text* text = static_cast<Text*>(node.get());
>          uint32_t textLength = text->TextLength();
> +        text->DeleteData(0, remainLength, IgnoreErrors());

Random side comment: I really like how the IgnoreErrors() reads in this code - it's so much more clear than the old way of doing it.

Comment 23

[Boris Zbarsky [:bzbarsky]]

Attachment: part 6.5. Fix some old whitespace bits in range code

MozReview-Commit-ID: KTEgqc4Wzjd

Comment 24

[Nika Layzell [:nika] (ni? for response)]

Comment on attachment 8959919 [details] [diff] [review]
part 12.  Remove nsIDOMCharacterData

Review of attachment 8959919 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/base/DocumentType.cpp
@@ +73,2 @@
>    // data nodes (they have a null .nodeValue and don't implement
> +  // CharacterData)

This reads really confusingly

Comment 25

[Nika Layzell [:nika] (ni? for response)]

Comment on attachment 8960306 [details] [diff] [review]
part 6.5.  Fix some old whitespace bits in range code

Review of attachment 8960306 [details] [diff] [review]:
-----------------------------------------------------------------

thanks

Comment 26

[Boris Zbarsky [:bzbarsky]]

> trailing whitespace.

Fixed.

> I just love how consistent our C++ style is in gecko.

Mmm... This is kin code.  Very old.  I will just fix this.  ;)

> Should this method be made to forward to InsertData?

Good point.  Done.

> trailing whitespace.

Fixed.

> I have a feeling you copy-pasted the same function a bunch of times :p

Well, the same failure check...  Fixed.

> I really like how the IgnoreErrors() reads in this code

Yeah, that part felt nice.  ;)

> This reads really confusingly

I will make it clearer that this refers to the DOM interface.

Comment 27

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/899edac390aa
part 1.  Rename nsGenericDOMDataNode to CharacterData.  r=mystor
https://hg.mozilla.org/integration/mozilla-inbound/rev/77630b78e554
part 2.  Rename nsGenericDOMDataNode.{h,cpp} to CharacterData.  r=mystor
https://hg.mozilla.org/integration/mozilla-inbound/rev/e43988b4ba26
part 3.  Remove nsIDOMCharacterData::Get/SetData.  r=mystor
https://hg.mozilla.org/integration/mozilla-inbound/rev/0cd517334cd5
part 4.  Remove nsIDOMCharacterData::SubstringData.  r=mystor
https://hg.mozilla.org/integration/mozilla-inbound/rev/87162a82c062
part 5.  Remove nsIDOMCharacterData::AppendData.  r=mystor
https://hg.mozilla.org/integration/mozilla-inbound/rev/1182d32f823e
part 6.  Remove nsIDOMCharacterData::GetLength  r=mystor
https://hg.mozilla.org/integration/mozilla-inbound/rev/c4984d2a063a
part 6.5.  Fix some old whitespace bits in range code.  r=mystor
https://hg.mozilla.org/integration/mozilla-inbound/rev/3e3ac3c3b989
part 7.  Remove nsIDOMCharacterData::InsertData.  r=mystor
https://hg.mozilla.org/integration/mozilla-inbound/rev/e429f3715268
part 8.  Remove nsIDOMCharacterData::DeleteData.  r=mystor
https://hg.mozilla.org/integration/mozilla-inbound/rev/b10a7fadaee8
part 9.  Remove nsIDOMCharacterData::ReplaceData.  r=mystor
https://hg.mozilla.org/integration/mozilla-inbound/rev/6525e7ba0b1d
part 10.  Remove remaining nsIDOMCharacterData uses in editor.  r=mystor
https://hg.mozilla.org/integration/mozilla-inbound/rev/18e3eb86e416
part 11.  Remove remaining nsIDOMCharacterData uses in range code.  r=mystor
https://hg.mozilla.org/integration/mozilla-inbound/rev/22f71b21eb19
part 12.  Remove nsIDOMCharacterData.  r=mystor

Comment 28

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6b72f16720e5
part 7 typo fix to reopen the CLOSED TREE.  r=bzbarsky

Comment 29

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/899edac390aa
https://hg.mozilla.org/mozilla-central/rev/77630b78e554
https://hg.mozilla.org/mozilla-central/rev/e43988b4ba26
https://hg.mozilla.org/mozilla-central/rev/0cd517334cd5
https://hg.mozilla.org/mozilla-central/rev/87162a82c062
https://hg.mozilla.org/mozilla-central/rev/1182d32f823e
https://hg.mozilla.org/mozilla-central/rev/c4984d2a063a
https://hg.mozilla.org/mozilla-central/rev/3e3ac3c3b989
https://hg.mozilla.org/mozilla-central/rev/e429f3715268
https://hg.mozilla.org/mozilla-central/rev/b10a7fadaee8
https://hg.mozilla.org/mozilla-central/rev/6525e7ba0b1d
https://hg.mozilla.org/mozilla-central/rev/18e3eb86e416
https://hg.mozilla.org/mozilla-central/rev/22f71b21eb19
https://hg.mozilla.org/mozilla-central/rev/6b72f16720e5