Closed Bug 1447889 Opened 2 years ago Closed 2 years ago

Make nsIDOMRange an empty interface

Categories

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

enhancement

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox61 --- fixed

People

(Reporter: bzbarsky, Assigned: bzbarsky)

References

Details

Attachments

(14 files)

6.81 KB, patch
Nika
: review+
Details | Diff | Splinter Review
11.77 KB, patch
Nika
: review+
Details | Diff | Splinter Review
8.10 KB, patch
Nika
: review+
Details | Diff | Splinter Review
55.08 KB, patch
Nika
: review+
Details | Diff | Splinter Review
7.51 KB, patch
Nika
: review+
Details | Diff | Splinter Review
45.34 KB, patch
Nika
: review+
Details | Diff | Splinter Review
4.77 KB, patch
Nika
: review+
Details | Diff | Splinter Review
9.77 KB, patch
Nika
: review+
Details | Diff | Splinter Review
13.51 KB, patch
Nika
: review+
Details | Diff | Splinter Review
3.12 KB, patch
Nika
: review+
Details | Diff | Splinter Review
11.56 KB, patch
Nika
: review+
Details | Diff | Splinter Review
5.86 KB, patch
Nika
: review+
Details | Diff | Splinter Review
13.05 KB, patch
Nika
: review+
Details | Diff | Splinter Review
12.97 KB, patch
Nika
: review+
Details | Diff | Splinter Review
No description provided.
Blocks: 746600
MozReview-Commit-ID: B8HePBcalWU
Attachment #8961233 - Flags: review?(nika)
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
It's unused; we never create these things by contract or classid.

MozReview-Commit-ID: 3Jsyb6QHlJt
Attachment #8961234 - Flags: review?(nika)
MozReview-Commit-ID: 2PYuoa1PFKL
Attachment #8961235 - Flags: review?(nika)
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
Attachment #8961236 - Flags: review?(nika)
MozReview-Commit-ID: 18bnPYjRld5
Attachment #8961237 - Flags: review?(nika)
MozReview-Commit-ID: 8yOZMWBexsN
Attachment #8961238 - Flags: review?(nika)
MozReview-Commit-ID: LLiXK8IpUdY
Attachment #8961239 - Flags: review?(nika)
MozReview-Commit-ID: 2hbF6pT31Xd
Attachment #8961240 - Flags: review?(nika)
MozReview-Commit-ID: 29swD9AoqoF
Attachment #8961241 - Flags: review?(nika)
MozReview-Commit-ID: IoXz0pS6zAa
Attachment #8961242 - Flags: review?(nika)
MozReview-Commit-ID: DAhBVEiYON7
Attachment #8961243 - Flags: review?(nika)
MozReview-Commit-ID: 8TDYC3f4ENn
Attachment #8961244 - Flags: review?(nika)
MozReview-Commit-ID: CjtfHTtcviJ
Attachment #8961245 - Flags: review?(nika)
Blocks: 1447890
MozReview-Commit-ID: JWJWGzY45ac
Attachment #8961246 - Flags: review?(nika)
Depends on: 1447121
Priority: -- → P2
Attachment #8961233 - Flags: review?(nika) → review+
Attachment #8961234 - Flags: review?(nika) → review+
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...
Attachment #8961235 - Flags: review?(nika) → review+
Oh, I had a direct cast there (with comment), then found the method but forgot to remove the comment.  ;)
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?
Attachment #8961236 - Flags: review?(nika) → review+
Attachment #8961237 - Flags: review?(nika) → review+
> Do you need to include both nsIDOMNode and nsINode?

Hmm, I guess nsINode includes nsIDOMNode.  I'll nix the nsIDOMNode include.
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)
Attachment #8961238 - Flags: review?(nika) → review+
Attachment #8961239 - Flags: review?(nika) → review+
Attachment #8961240 - Flags: review?(nika) → review+
Attachment #8961241 - Flags: review?(nika) → review+
Attachment #8961242 - Flags: review?(nika) → review+
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()?
Attachment #8961243 - Flags: review?(nika) → review+
Attachment #8961244 - Flags: review?(nika) → review+
Attachment #8961245 - Flags: review?(nika) → review+
Attachment #8961246 - Flags: review?(nika) → review+
> 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.
Pushed by bzbarsky@mozilla.com:
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
Duplicate of this bug: 746600
Depends on: 1449601
You need to log in before you can comment on or make changes to this bug.