Last Comment Bug 752203 - Cleanup nsEditor::NodesSameType
: Cleanup nsEditor::NodesSameType
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Editor (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla15
Assigned To: :Ms2ger
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-05-05 05:38 PDT by :Ms2ger
Modified: 2012-05-18 02:48 PDT (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch v1 (5.77 KB, patch)
2012-05-05 05:38 PDT, :Ms2ger
no flags Details | Diff | Review
Patch v1.1 (5.82 KB, patch)
2012-05-05 05:48 PDT, :Ms2ger
ehsan: review-
Details | Diff | Review
Patch v2 (5.82 KB, patch)
2012-05-13 02:50 PDT, :Ms2ger
ehsan: review+
Details | Diff | Review

Description :Ms2ger 2012-05-05 05:38:17 PDT
Created attachment 621284 [details] [diff] [review]
Patch v1
Comment 1 :Ms2ger 2012-05-05 05:48:37 PDT
Created attachment 621286 [details] [diff] [review]
Patch v1.1

Now it also builds.
Comment 2 :Ehsan Akhgari (out sick) 2012-05-07 08:01:03 PDT
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.
Comment 3 :Ms2ger 2012-05-07 08:08:18 PDT
Me neither, but I wanted to preserve behaviour (both GetTag's would return null in that case, so they would compare equal)
Comment 4 :Ehsan Akhgari (out sick) 2012-05-07 08:10:19 PDT
(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.  :-)
Comment 5 :Ms2ger 2012-05-07 08:13:38 PDT
wfm
Comment 6 :Ms2ger 2012-05-13 02:50:03 PDT
Created attachment 623491 [details] [diff] [review]
Patch v2

Note You need to log in before you can comment on or make changes to this bug.