Init method of nsIDocumentEncoder does not reset member variables

VERIFIED FIXED

Status

()

--
major
VERIFIED FIXED
14 years ago
14 years ago

People

(Reporter: David.R.Gardiner, Unassigned)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

14 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.5) Gecko/20041107 Firefox/1.0
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b) Gecko/20050116

If (as I had assumed) calling the Init method of classes inheriting from
nsIDocumentEncoder (eg. nsHTMLCopyEncoder) a second time to reuse an existing
object, you experience a bug caused by some of the member variables only being
initialised inside the constructor.

I believe this is the root cause of bug 278438 

Reproducible: Always

Actual Results:  
Calling Init(..) a second time on an existing nsIDocumentEncoder object then
calling another method eg. SetSelection or EncodeToStringWithContext returns
unexpected results

Expected Results:  
Calling Init(..) should reset all internal member variables so that the object
can be reused without worrying about previous uses.
(Reporter)

Updated

14 years ago
Blocks: 278438
(Reporter)

Comment 1

14 years ago
Created attachment 171505 [details] [diff] [review]
Reset member variables when Init method is called

Dean,
this fixes a problem that has occurred after my SourceURL Copy/Paste bug 244685
patch landed.

I have a workaround patch in bug 278438 for the code that experiences the
problem, but this patch would actually solve the problem properly.

thanks,
-dave
(Reporter)

Updated

14 years ago
Attachment #171505 - Flags: review?(dean_tessman)

Comment 2

14 years ago
Comment on attachment 171505 [details] [diff] [review]
Reset member variables when Init method is called

This looks fairly straight-forward to me.

>@@ -1064,19 +1073,23 @@ nsHTMLCopyEncoder::~nsHTMLCopyEncoder()
> NS_IMETHODIMP
> nsHTMLCopyEncoder::Init(nsIDocument* aDocument,
>                         const nsAString& aMimetype,
>                         PRUint32 aFlags)
> {
>   if (!aDocument)
>     return NS_ERROR_INVALID_ARG;
> 
>+  mIsTextWidget = PR_FALSE;
>+  Initialize();

Can't mIsTextWidget be set to PR_FALSE in the base Initialize()?  Any place
that sets it to PR_TRUE is called directly.

Also, does this negate the change that was made for bug 273741, or is that
needed regardless?

r=me with those questions answered
Attachment #171505 - Flags: review?(dean_tessman) → review+
(Reporter)

Comment 3

14 years ago
(In reply to comment #2)
> Can't mIsTextWidget be set to PR_FALSE in the base Initialize()?  Any place
> that sets it to PR_TRUE is called directly.

mIsTextWidget is only declared in the nsHTMLCopyEncoder (not in
nsDocumentEncoder where i put the Initialize method). I guess I could add an
override method in the subclass - what do you think?
 
> Also, does this negate the change that was made for bug 273741, or is that
> needed regardless?

I think that change is still needed - there was a flaw in how I handled the
plaintext vs. HTML format stuff that this patch wouldn't solve.

-dave
(Reporter)

Comment 4

14 years ago
Comment on attachment 171505 [details] [diff] [review]
Reset member variables when Init method is called

Neil,
this fixes a problem has appeared after my SourceURL Copy/Paste bug 244685
patch landed.

The Init() method didn't reset everything when it was called a second (or more)
times.

-dave
Attachment #171505 - Flags: superreview?(neil.parkwaycc.co.uk)
(Reporter)

Comment 5

14 years ago
Should I set any of the blocking flags on this?

-dave

Comment 6

14 years ago
Comment on attachment 171505 [details] [diff] [review]
Reset member variables when Init method is called

Sorry, not my area, please check the list at
http://www.mozilla.org/hacking/reviewers.html for a more suitable victim.
Attachment #171505 - Flags: superreview?(neil.parkwaycc.co.uk) → superreview-

Comment 7

14 years ago
Sorry, I mean to cancel that request, not deny it :-[
(Reporter)

Updated

14 years ago
Attachment #171505 - Flags: superreview- → superreview?(bzbarsky)
(Reporter)

Updated

14 years ago
Attachment #171505 - Flags: superreview?(bzbarsky)
(Reporter)

Updated

14 years ago
Attachment #171505 - Flags: superreview?(jst)
Comment on attachment 171505 [details] [diff] [review]
Reset member variables when Init method is called

sr=jst
Attachment #171505 - Flags: superreview?(jst) → superreview+

Comment 9

14 years ago
Comment on attachment 171505 [details] [diff] [review]
Reset member variables when Init method is called

Checking in nsDocumentEncoder.cpp;
/cvsroot/mozilla/content/base/src/nsDocumentEncoder.cpp,v  <-- 
nsDocumentEncoder.cpp
new revision: 1.104; previous revision: 1.103
done
Verified FIXED with the 2005-01-21-05 Windows XP Seamonkey trunk builds using my
testcase in bug 278438.  I'll go ahead and just mark that bug a straight DUP of
this.

Thanks, David, and sorry for jumping the gun on the patch commit yesterday.
Status: NEW → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED
*** Bug 278438 has been marked as a duplicate of this bug. ***
Now verifying.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.