remove usage of nsISelectionPrivate.getEnumerator()

RESOLVED FIXED in mozilla21

Status

()

defect
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: tbsaunde, Assigned: tbsaunde)

Tracking

unspecified
mozilla21
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Assignee

Description

6 years ago
No description provided.
Assignee

Comment 1

6 years ago
Posted patch patch (obsolete) — Splinter Review
no super hurry here, its already been lying around in my queue for a week
Attachment #711312 - Flags: review?(bugs)
Assignee

Updated

6 years ago
Depends on: 839059
Comment on attachment 711312 [details] [diff] [review]
patch

This patch needs to be updated once we have builtinclass nsISelection.
Attachment #711312 - Flags: review?(bugs)
Assignee

Comment 3

6 years ago
Posted patch patch2Splinter Review
Attachment #711312 - Attachment is obsolete: true
Attachment #712580 - Flags: review?(bugs)
Comment on attachment 712580 [details] [diff] [review]
patch2

>+  for (uint32_t rangeIdx = 0; rangeIdx < rangeCount; rangeIdx++) {
s/rangeIdx++/++rangeIdx/


>+++ b/content/media/webaudio/MediaBufferDecoder.cpp
>@@ -121,16 +121,18 @@ private:
>   // This monitor object is not really used to synchronize access to anything.
>   // It's just there in order for us to be able to override
>   // GetReentrantMonitor correctly.
>   ReentrantMonitor mReentrantMonitor;
>   nsCOMPtr<nsIThread> mDecodeThread;
>   nsAutoPtr<MediaResource> mResource;
> };
> 
>+using namespace mozilla::dom;
I doubt this change is related to this bug


>@@ -2303,32 +2302,23 @@ nsHTMLEditRules::WillDeleteSelection(Selection* aSelection,
...
>+        uint32_t rangeCount = aSelection->GetRangeCount();
>+        for (uint32_t rangeIdx = 0; rangeIdx < rangeCount; rangeIdx++) {
++rangeIdx

>@@ -5773,35 +5763,25 @@ nsHTMLEditRules::GetListActionNodes(nsCOMArray<nsIDOMNode> &outArrayOfNodes,
>+    uint32_t rangeCount = sel->GetRangeCount();
>+    for (uint32_t rangeIdx = 0; rangeIdx < rangeCount; rangeIdx++) {
++rangeIdx


>+  Selection* sel = static_cast<Selection*>(selection.get());
>+  uint32_t rangeCount = sel->GetRangeCount();
>+  for (uint32_t rangeIdx = 0; rangeIdx < rangeCount; rangeIdx++) {
++rangeIdx


>+    nsRange* range = sel->GetRangeAt(rangeIdx);
Could you keep this nsRefPtr<nsRange> so that we don't change the behavior in this bug.

>@@ -2383,17 +2382,17 @@ nsHTMLEditor::GetSelectedElement(const nsAString& aTagName, nsIDOMElement** aRet
>   // default is null - no element found
>   *aReturn = nullptr;
>   
>   // First look for a single element in selection
>   nsCOMPtr<nsISelection>selection;
>   nsresult res = GetSelection(getter_AddRefs(selection));
>   NS_ENSURE_SUCCESS(res, res);
>   NS_ENSURE_TRUE(selection, NS_ERROR_NULL_POINTER);
>-  nsCOMPtr<nsISelectionPrivate> selPriv(do_QueryInterface(selection));
>+  Selection* sel = static_cast<Selection*>(selection.get());
make this nsRefPtr<Selection>, since it is not clear to me whether something in the method may end up
killing selection object.

>       } else {
>-        printf("Could not create enumerator for GetSelectionProperties\n");
>+        // Should never get here?
>+        isCollapsed = true;
>+        printf("isCollapsed was FALSE, but no elements found in selection\n");
Make this NS_WARNING


>-      nsCOMPtr<nsIDOMRange> range( do_QueryInterface(currentItem) );
>+    uint32_t rangeCount = selection->GetRangeCount();
>+    for (uint32_t rangeIdx = 0; rangeIdx < rangeCount; rangeIdx++) {
++rangeIdx

>+    uint32_t rangeCount = selection->GetRangeCount();
>+    for (uint32_t rangeIdx = 0; rangeIdx < rangeCount; rangeIdx++) {
++rangeIdx


>@@ -1110,33 +1097,25 @@ nsHTMLEditor::GetInlinePropertyBase(nsIAtom *aProperty,
>   *aAll = true;
>   *aFirst = false;
>   bool first = true;
> 
>   nsCOMPtr<nsISelection> selection;
>   result = GetSelection(getter_AddRefs(selection));
>   NS_ENSURE_SUCCESS(result, result);
>   NS_ENSURE_TRUE(selection, NS_ERROR_NULL_POINTER);
>-  nsCOMPtr<nsISelectionPrivate> selPriv(do_QueryInterface(selection));
>+  Selection* sel = static_cast<Selection*>(selection.get());
Maybe nsRefPtr<Selection>
..
>+    uint32_t rangeCount = selection->GetRangeCount();
>+    for (uint32_t rangeIdx = 0; rangeIdx < rangeCount; rangeIdx++) {
++rangeIdx
Same still elsewhere.
Attachment #712580 - Flags: review?(bugs) → review+
Assignee

Comment 5

6 years ago
> >+  for (uint32_t rangeIdx = 0; rangeIdx < rangeCount; rangeIdx++) {
> s/rangeIdx++/++rangeIdx/

its odd style but ok

> >+++ b/content/media/webaudio/MediaBufferDecoder.cpp
> >@@ -121,16 +121,18 @@ private:
> >   // This monitor object is not really used to synchronize access to anything.
> >   // It's just there in order for us to be able to override
> >   // GetReentrantMonitor correctly.
> >   ReentrantMonitor mReentrantMonitor;
> >   nsCOMPtr<nsIThread> mDecodeThread;
> >   nsAutoPtr<MediaResource> mResource;
> > };
> > 
> >+using namespace mozilla::dom;
> I doubt this change is related to this bug

yeah, it was odd local build bustage that crept into this patch

> >@@ -2383,17 +2382,17 @@ nsHTMLEditor::GetSelectedElement(const nsAString& aTagName, nsIDOMElement** aRet
> >   // default is null - no element found
> >   *aReturn = nullptr;
> >   
> >   // First look for a single element in selection
> >   nsCOMPtr<nsISelection>selection;
> >   nsresult res = GetSelection(getter_AddRefs(selection));
> >   NS_ENSURE_SUCCESS(res, res);
> >   NS_ENSURE_TRUE(selection, NS_ERROR_NULL_POINTER);
> >-  nsCOMPtr<nsISelectionPrivate> selPriv(do_QueryInterface(selection));
> >+  Selection* sel = static_cast<Selection*>(selection.get());
> make this nsRefPtr<Selection>, since it is not clear to me whether something
> in the method may end up
> killing selection object.

same as below given there's already a refptr pointing at the selection I don't see how its possible for it to die

> >@@ -1110,33 +1097,25 @@ nsHTMLEditor::GetInlinePropertyBase(nsIAtom *aProperty,
> >   *aAll = true;
> >   *aFirst = false;
> >   bool first = true;
> > 
> >   nsCOMPtr<nsISelection> selection;
> >   result = GetSelection(getter_AddRefs(selection));
> >   NS_ENSURE_SUCCESS(result, result);
> >   NS_ENSURE_TRUE(selection, NS_ERROR_NULL_POINTER);
> >-  nsCOMPtr<nsISelectionPrivate> selPriv(do_QueryInterface(selection));
> >+  Selection* sel = static_cast<Selection*>(selection.get());
> Maybe nsRefPtr<Selection>

until the nsCOMPtr<nsISelection> goes away I don't see what that would effect since they always point at the same thing.
https://hg.mozilla.org/mozilla-central/rev/b533e53cea59
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
You need to log in before you can comment on or make changes to this bug.