1447889 - Make nsIDOMRange an empty interface

Status: RESOLVED FIXED

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

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: part 1. Change nsCopySupport to work with Selection a bit more

MozReview-Commit-ID: B8HePBcalWU

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: part 2. Remove the XPCOM goop for creating Selection

It's unused; we never create these things by contract or classid.

MozReview-Commit-ID: 3Jsyb6QHlJt

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: part 3. Use Selection and nsRange more in nsDocumentEncoder

MozReview-Commit-ID: 2PYuoa1PFKL

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: part 4. Remove nsIDOMRange::GetStart/EndContainer/Offset

I got a bit carried away with fixing up consumers to use nsINode...  But as a
result removing these methods all together made sense.

MozReview-Commit-ID: G47Cx1AijXT

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: part 5. Remove nsIDOMRange::GetCommonAncestorContainer

MozReview-Commit-ID: 18bnPYjRld5

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: part 6. Remove nsIDOMRange::SetStart/End

MozReview-Commit-ID: 8yOZMWBexsN

Comment 7

[Boris Zbarsky [:bzbarsky]]

Attachment: part 7. Remove nsIDOMRange::Collapse

MozReview-Commit-ID: LLiXK8IpUdY

Comment 8

[Boris Zbarsky [:bzbarsky]]

Attachment: part 8. Remove nsIDOMRange::SelectNode/SelectNodeContents

MozReview-Commit-ID: 2hbF6pT31Xd

Comment 9

[Boris Zbarsky [:bzbarsky]]

Attachment: part 9. Remove nsIDOMRange::CompareBoundaryPoints

MozReview-Commit-ID: 29swD9AoqoF

Comment 10

[Boris Zbarsky [:bzbarsky]]

Attachment: part 10. Remove nsIDOMRange::DeleteContents

MozReview-Commit-ID: IoXz0pS6zAa

Comment 11

[Boris Zbarsky [:bzbarsky]]

Attachment: part 11. Remove nsIDOMRange::CloneRange

MozReview-Commit-ID: DAhBVEiYON7

Comment 12

[Boris Zbarsky [:bzbarsky]]

Attachment: part 12. Remove nsIDOMRange::ToString

MozReview-Commit-ID: 8TDYC3f4ENn

Comment 13

[Boris Zbarsky [:bzbarsky]]

Attachment: part 13. Remove unused nsIDOMRange bits

MozReview-Commit-ID: CjtfHTtcviJ

Comment 14

[Boris Zbarsky [:bzbarsky]]

Attachment: part 14. Remove mention of nsIDOMRange from layout/

MozReview-Commit-ID: JWJWGzY45ac

Comment 15

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

Comment on attachment 8961235 [details] [diff] [review]
part 3.  Use Selection and nsRange more in nsDocumentEncoder

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

::: dom/base/nsDocumentEncoder.cpp
@@ +282,5 @@
>  
>  NS_IMETHODIMP
>  nsDocumentEncoder::SetSelection(nsISelection* aSelection)
>  {
> +  // Safe because nsISelection is builtinclass.

This is a method on the type rather than a direct cast, it feels like this comment would be on the nsISelection object rather than here...

Comment 16

[Boris Zbarsky [:bzbarsky]]

Oh, I had a direct cast there (with comment), then found the method but forgot to remove the comment.  ;)

Comment 17

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

Comment on attachment 8961236 [details] [diff] [review]
part 4.  Remove nsIDOMRange::GetStart/EndContainer/Offset

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

::: toolkit/components/find/nsFind.h
@@ +11,5 @@
>  
>  #include "nsCOMPtr.h"
>  #include "nsCycleCollectionParticipant.h"
>  #include "nsIDOMNode.h"
> +#include "nsINode.h"

Do you need to include both nsIDOMNode and nsINode?

Comment 18

[Boris Zbarsky [:bzbarsky]]

> Do you need to include both nsIDOMNode and nsINode?

Hmm, I guess nsINode includes nsIDOMNode.  I'll nix the nsIDOMNode include.

Comment 19

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

Comment on attachment 8961238 [details] [diff] [review]
part 6.  Remove nsIDOMRange::SetStart/End

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

::: dom/base/nsDocumentEncoder.cpp
@@ +1567,5 @@
>  
>  nsresult
> +nsHTMLCopyEncoder::GetPromotedPoint(Endpoint aWhere, nsINode *aNode,
> +                                    int32_t aOffset, nsCOMPtr<nsINode> *outNode,
> +                                    int32_t *outOffset, nsINode *common)

If you're rewriting the signature like this, want to move the * to the type?

(I guess this file follows the other convention - so it doesn't really matter)

Comment 20

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

Comment on attachment 8961243 [details] [diff] [review]
part 11.  Remove nsIDOMRange::CloneRange

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

::: toolkit/components/typeaheadfind/nsTypeAheadFind.cpp
@@ +1141,5 @@
>  
>  void
>  nsTypeAheadFind::GetSelection(nsIPresShell *aPresShell,
>                                nsISelectionController **aSelCon,
> +                              Selection **aDOMSel)

What does this change (and all of the GetSelection-related ones) have to do with the commit message?

@@ +1157,5 @@
>    if (presContext && frame) {
>      frame->GetSelectionController(presContext, aSelCon);
>      if (*aSelCon) {
> +      *aDOMSel =
> +        (*aSelCon)->GetDOMSelection(nsISelectionController::SELECTION_NORMAL);

Can you just use a RefPtr here, or do_AddRef().take()?

Comment 21

[Boris Zbarsky [:bzbarsky]]

> If you're rewriting the signature like this, want to move the * to the type?

Sure, done.

> What does this change (and all of the GetSelection-related ones) have to do with the commit message?

I am switching some nsISelection to Selection, so I can get and nsRange out of it via GetRangeAt and then call nsRange::CloneRange.  I could have broken this up into more patches, I guess...

I'll add this to the commit message.

> Can you just use a RefPtr here, or do_AddRef().take()?

Can do, with RefPtr.

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6c4b0fd365f2
part 1.  Change nsCopySupport to work with Selection a bit more.  r=mystor
https://hg.mozilla.org/integration/mozilla-inbound/rev/2ca7872fa5b6
part 2.  Remove the XPCOM goop for creating Selection.  r=mystor
https://hg.mozilla.org/integration/mozilla-inbound/rev/85a5cd611076
part 3.  Use Selection and nsRange more in nsDocumentEncoder.  r=mystor
https://hg.mozilla.org/integration/mozilla-inbound/rev/a4caa4937a31
part 4.  Remove nsIDOMRange::GetStart/EndContainer/Offset.  r=mystor
https://hg.mozilla.org/integration/mozilla-inbound/rev/063c747e3f31
part 5.  Remove nsIDOMRange::GetCommonAncestorContainer.  r=mystor
https://hg.mozilla.org/integration/mozilla-inbound/rev/a55c7ad25f9a
part 6.  Remove nsIDOMRange::SetStart/End.  r=mystor
https://hg.mozilla.org/integration/mozilla-inbound/rev/37568a081a19
part 7.  Remove nsIDOMRange::Collapse.  r=mystor
https://hg.mozilla.org/integration/mozilla-inbound/rev/f9403cedcdb7
part 8.  Remove nsIDOMRange::SelectNode/SelectNodeContents.  r=mystor
https://hg.mozilla.org/integration/mozilla-inbound/rev/5e13b554508b
part 9.  Remove nsIDOMRange::CompareBoundaryPoints.  r=mystor
https://hg.mozilla.org/integration/mozilla-inbound/rev/0909753f32af
part 10. Remove nsIDOMRange::DeleteContents. r=mystor
https://hg.mozilla.org/integration/mozilla-inbound/rev/4abb3e23e0c5
part 11.  Remove nsIDOMRange::CloneRange.  r=mystor
https://hg.mozilla.org/integration/mozilla-inbound/rev/dedb54f89b3d
part 12.  Remove nsIDOMRange::ToString.  r=mystor
https://hg.mozilla.org/integration/mozilla-inbound/rev/e97a30657dfe
part 13.  Remove unused nsIDOMRange bits.  r=mystor
https://hg.mozilla.org/integration/mozilla-inbound/rev/d463dcaf44db
part 14.  Remove mention of nsIDOMRange from layout/.  r=mystor

Comment 23

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/6c4b0fd365f2
https://hg.mozilla.org/mozilla-central/rev/2ca7872fa5b6
https://hg.mozilla.org/mozilla-central/rev/85a5cd611076
https://hg.mozilla.org/mozilla-central/rev/a4caa4937a31
https://hg.mozilla.org/mozilla-central/rev/063c747e3f31
https://hg.mozilla.org/mozilla-central/rev/a55c7ad25f9a
https://hg.mozilla.org/mozilla-central/rev/37568a081a19
https://hg.mozilla.org/mozilla-central/rev/f9403cedcdb7
https://hg.mozilla.org/mozilla-central/rev/5e13b554508b
https://hg.mozilla.org/mozilla-central/rev/0909753f32af
https://hg.mozilla.org/mozilla-central/rev/4abb3e23e0c5
https://hg.mozilla.org/mozilla-central/rev/dedb54f89b3d
https://hg.mozilla.org/mozilla-central/rev/e97a30657dfe
https://hg.mozilla.org/mozilla-central/rev/d463dcaf44db