Last Comment Bug 716215 - nsHTMLEditor::IsEmptyNodeImpl checks if the parent is a form widget instead of the child
: nsHTMLEditor::IsEmptyNodeImpl checks if the parent is a form widget instead o...
Status: RESOLVED FIXED
[good first bug][mentor=ehsan][lang=c++]
:
Product: Core
Classification: Components
Component: Editor (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla12
Assigned To: Jignesh Kakadiya [:jhk]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-01-07 04:10 PST by :Ms2ger
Modified: 2012-01-11 05:02 PST (History)
3 users (show)
Ms2ger: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (1.19 KB, patch)
2012-01-10 04:08 PST, Jignesh Kakadiya [:jhk]
Ms2ger: review+
ehsan: superreview+
Details | Diff | Splinter Review

Description :Ms2ger 2012-01-07 04:10:33 PST
<http://mxr.mozilla.org/mozilla-central/source/editor/libeditor/html/nsHTMLEditor.cpp#4958> has

4921   while (child)
4923     nsCOMPtr<nsIDOMNode> node = child;
4959           else if (nsHTMLEditUtils::IsFormWidget(aNode))
4960           { // break out if we find we aren't empty
4961             *outIsEmptyNode = false;
4962             return NS_OK;
4963           }

where it probably wants to check nsHTMLEditUtils::IsFormWidget(node). Ehsan, could you confirm?
Comment 1 :Ms2ger 2012-01-07 04:39:20 PST
In fact, there's an early return for nsHTMLEditUtils::IsFormWidget(aNode) at line 4904.
Comment 2 :Ehsan Akhgari 2012-01-09 16:09:36 PST
(In reply to Ms2ger from comment #0)
> <http://mxr.mozilla.org/mozilla-central/source/editor/libeditor/html/
> nsHTMLEditor.cpp#4958> has
> 
> 4921   while (child)
> 4923     nsCOMPtr<nsIDOMNode> node = child;
> 4959           else if (nsHTMLEditUtils::IsFormWidget(aNode))
> 4960           { // break out if we find we aren't empty
> 4961             *outIsEmptyNode = false;
> 4962             return NS_OK;
> 4963           }
> 
> where it probably wants to check nsHTMLEditUtils::IsFormWidget(node). Ehsan,
> could you confirm?

Yes, you're right.
Comment 3 Jignesh Kakadiya [:jhk] 2012-01-10 04:08:36 PST
Created attachment 587274 [details] [diff] [review]
Patch
Comment 4 :Ms2ger 2012-01-10 05:36:06 PST
Comment on attachment 587274 [details] [diff] [review]
Patch

Thanks, looks good.
Comment 5 :Ehsan Akhgari 2012-01-10 06:53:36 PST
Comment on attachment 587274 [details] [diff] [review]
Patch

Looks great, Jignesh, thanks a lot!  Do you have try server access?  Have you tested this patch on the try server?
Comment 6 Jignesh Kakadiya [:jhk] 2012-01-10 07:31:24 PST
> Do you have try server access?  Have
> you tested this patch on the try server?

No. I have applied for try access and will get it soon :).

Thanks!
Comment 8 :Ms2ger 2012-01-11 05:02:54 PST
Thanks for the patch!

https://hg.mozilla.org/mozilla-central/rev/c42d08fdec34

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