Cleanup nsEditor::NodesSameType

RESOLVED FIXED in mozilla15

Status

()

Core
Editor
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Ms2ger, Assigned: Ms2ger)

Tracking

Trunk
mozilla15
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

5 years ago
Created attachment 621284 [details] [diff] [review]
Patch v1
Attachment #621284 - Flags: review?(ehsan)
(Assignee)

Comment 1

5 years ago
Created attachment 621286 [details] [diff] [review]
Patch v1.1

Now it also builds.
Attachment #621284 - Attachment is obsolete: true
Attachment #621284 - Flags: review?(ehsan)
Attachment #621286 - Flags: review?(ehsan)
Comment on attachment 621286 [details] [diff] [review]
Patch v1.1

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

Looks good overall, r- because of the comment below.

::: editor/libeditor/base/nsEditor.cpp
@@ +3844,5 @@
> +
> +  nsCOMPtr<nsIContent> content1 = do_QueryInterface(aNode1);
> +  nsCOMPtr<nsIContent> content2 = do_QueryInterface(aNode2);
> +  if (!content1 || !content2) {
> +    return !content1 == !content2;

Hmm, is why do we want to do this?  This means that when content1 and content2 are both null, we'll consider them to be the same type.  I don't see why that makes sense.
Attachment #621286 - Flags: review?(ehsan) → review-
(Assignee)

Comment 3

5 years ago
Me neither, but I wanted to preserve behaviour (both GetTag's would return null in that case, so they would compare equal)
(In reply to Ms2ger from comment #3)
> Me neither, but I wanted to preserve behaviour (both GetTag's would return
> null in that case, so they would compare equal)

Hmm, then I think the existing behavior is badly broken, let's fix it.  :-)
(Assignee)

Comment 5

5 years ago
wfm
(Assignee)

Comment 6

5 years ago
Created attachment 623491 [details] [diff] [review]
Patch v2
Attachment #621286 - Attachment is obsolete: true
Attachment #623491 - Flags: review?(ehsan)
Attachment #623491 - Flags: review?(ehsan) → review+
(Assignee)

Comment 7

5 years ago
https://hg.mozilla.org/mozilla-central/rev/f7d7fad55752
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla15
You need to log in before you can comment on or make changes to this bug.