Last Comment Bug 178790 - separate xpcom wrapper and charset detector
: separate xpcom wrapper and charset detector
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Internationalization (show other bugs)
: Trunk
: x86 All
: -- normal (vote)
: ---
Assigned To: Jungshik Shin
: Yuying Long
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2002-11-06 19:47 PST by Shanjian Li
Modified: 2004-08-02 21:14 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (17.70 KB, patch)
2002-11-15 13:47 PST, Shanjian Li
alecf: superreview+
Details | Diff | Review
Adding the new file to the project. (2.47 KB, patch)
2002-12-02 13:14 PST, nhottanscp
alecf: superreview+
Details | Diff | Review
patch (addressing alecf's comment and changing license to MPL) (18.48 KB, patch)
2004-08-02 11:04 PDT, Jungshik Shin
smontagu: review+
jshin1987: superreview+
Details | Diff | Review

Description Shanjian Li 2002-11-06 19:47:17 PST
Unlike rest of the universal detector implementation file, nsUniversalDetector.h
and nsUniversalDetector.cpp contains both implementation class (which has no
dependency on XPCOM) and 2 XPCOM wrapper class. Separate those class to
different files will benefit maintenance and future reuse.
Comment 1 Shanjian Li 2002-11-15 13:47:29 PST
Created attachment 106410 [details] [diff] [review]
patch
Comment 2 Shanjian Li 2002-11-15 13:48:39 PST
patch
Comment 3 nhottanscp 2002-12-02 13:14:25 PST
Created attachment 107934 [details] [diff] [review]
Adding the new file to the project.
Comment 4 Alec Flett 2003-01-03 08:56:37 PST
Comment on attachment 106410 [details] [diff] [review]
patch 

- You don't need to NS_INIT_ISUPPORTS anymore
- Why not make mObserver an nsCOMPtr?

with those changes, sr=alecf
Comment 5 Alec Flett 2003-01-03 08:57:24 PST
Comment on attachment 107934 [details] [diff] [review]
Adding the new file to the project.

sr=alecf
Comment 6 Katsuhiko Momoi 2004-04-12 14:28:08 PDT
CC'ing jungshik to see what if any we should do about this bug.
Comment 7 Jean-Marc Desperrier 2004-08-02 09:12:52 PDT
Well why not just do the check-in ? There will very little update needed as the
code almost didn't change since the patch was written.

And it would make things easier for independant use of the detector, like for
example what I suggested at the end of bug 135762.
Comment 8 Jungshik Shin 2004-08-02 11:04:56 PDT
Created attachment 155006 [details] [diff] [review]
patch (addressing alecf's comment and changing license to MPL)

attachment 106410 [details] [diff] [review] needs a little update (it didn't get cleanly applied) in
addition to what alecf's pointed out. attachment 107934 [details] [diff] [review] is not needed any more.
Comment 9 Jungshik Shin 2004-08-02 11:08:31 PDT
Comment on attachment 155006 [details] [diff] [review]
patch (addressing alecf's comment and changing license to MPL)

asking for r (just in case).

Jean-Marc, thanks for reminding me of this (somehow I don't remember reading
Kat's comment)
Comment 10 Simon Montagu :smontagu 2004-08-02 12:57:46 PDT
Comment on attachment 155006 [details] [diff] [review]
patch (addressing alecf's comment and changing license to MPL)

>+NS_IMETHODIMP nsUniversalXPCOMDetector::DoIt(
>+  const char* aBuf, PRUint32 aLen, PRBool* oDontFeedMe)
>+{
>+  NS_ASSERTION(mObserver != nsnull , "have not init yet");
>+
>+  if((nsnull == aBuf) || (nsnull == oDontFeedMe))
>+     return NS_ERROR_ILLEGAL_VALUE;
>+
>+  this->HandleData(aBuf, aLen);

Make this:
    nsresult rv = this->HandleData(aBuf, aLen);
    if (NS_FAILED(rv))
      return rv;
(Change from bug 225994)

>+NS_IMETHODIMP nsUniversalXPCOMStringDetector::DoIt(const char* aBuf, PRUint32 aLen, 
>+                     const char** oCharset, nsDetectionConfident &oConf)
>+{
>+   mResult = nsnull;
>+   this->Reset();
>+   this->HandleData(aBuf, aLen); 

Make the corresponding change from bug 225994 here too. Also detabify the new
files and r=smontagu
Comment 11 Jungshik Shin 2004-08-02 21:14:33 PDT
patch checked in with Simon's concerns addressed. I changed the indentation a
bit more.

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