Closed Bug 752203 Opened 12 years ago Closed 12 years ago

Cleanup nsEditor::NodesSameType

Categories

(Core :: DOM: Editor, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla15

People

(Reporter: Ms2ger, Assigned: Ms2ger)

Details

Attachments

(1 file, 2 obsolete files)

Attached patch Patch v1 (obsolete) — Splinter Review
      No description provided.
Attachment #621284 - Flags: review?(ehsan)
Attached patch Patch v1.1 (obsolete) — Splinter Review
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-
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.  :-)
wfm
Attached patch Patch v2Splinter Review
Attachment #621286 - Attachment is obsolete: true
Attachment #623491 - Flags: review?(ehsan)
Attachment #623491 - Flags: review?(ehsan) → review+
https://hg.mozilla.org/mozilla-central/rev/f7d7fad55752
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla15
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: