Last Comment Bug 578443 - Block Internet Download Manager for Firefox 4.0b1, and more...
: Block Internet Download Manager for Firefox 4.0b1, and more...
Status: RESOLVED FIXED
Website fix, doesn't block code-compl...
:
Product: Toolkit
Classification: Components
Component: Blocklisting (show other bugs)
: unspecified
: x86 All
: P2 major with 1 vote (vote)
: 5.12
Assigned To: Michael Morgan [:morgamic]
:
Mentors:
https://addons.mozilla.org/en-US/fire...
: 530524 (view as bug list)
Depends on: 580393
Blocks: 563984 527339 564008 571103 578281
  Show dependency treegraph
 
Reported: 2010-07-13 12:36 PDT by chris hofmann
Modified: 2016-03-07 15:30 PST (History)
66 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
-


Attachments
updated blocklist.xml (2.90 KB, text/plain)
2010-07-21 12:57 PDT, Michael Morgan [:morgamic]
robert.strong.bugs: review-
Details
v2, adding 6.9.9 restriction (2.75 KB, text/plain)
2010-07-23 16:46 PDT, Michael Morgan [:morgamic]
robert.strong.bugs: review-
Details
v3, adding * for app range and changing item version to 6.9.8 (2.75 KB, patch)
2010-08-04 13:36 PDT, Michael Morgan [:morgamic]
robert.strong.bugs: review+
Details | Diff | Splinter Review
v4, changes old entry as well... (2.93 KB, patch)
2010-08-04 14:27 PDT, Michael Morgan [:morgamic]
robert.strong.bugs: review+
Details | Diff | Splinter Review
i need idm to use in this browser (140.50 KB, patch)
2012-03-14 13:22 PDT, jamdar
no flags Details | Diff | Splinter Review

Description chris hofmann 2010-07-13 12:36:47 PDT
It looks like Internet Download Manager might be making up between 25-30% of Firefox 4b1 crashes.

See the top crash list and these bugs

http://crash-stats.mozilla.com/topcrasher/byversion/Firefox/4.0b1

13.72 %      nsPurpleBuffer::SelectPointers(GCGraphBuilder&)  bug 564008
 3.27 %	     nsINode::GetFlags()  bug  563984
 2.85 %      nsCycleCollectingAutoRefCnt::unmarkPurple() bug 527339
 1.68 %      nsGenericElement::DestroyContent        bug 578281
 1.34 %      FindNamedItems

and many more signatures on the top crash list show 100% correlation to IDM

It sounds like we might not be able to block specific versions of IDM, but we should block all versions where its used with firefox 4.0 betas, until a new version gets out there.

users seem to be having a hard time understanding the correlation between there crashes and IDM and we are missing other good feedback on the beta from IDM users.
Comment 1 Jorge Villalobos [:jorgev] 2010-07-13 18:10:36 PDT
Agreed on blocklisting for anything above 3.6.*
Comment 2 Wil Clouser [:clouserw] 2010-07-14 11:57:55 PDT
Can we be specific about what add-on we are talking about?  Is it https://addons.mozilla.org/en-US/firefox/addon/6973/ ?
Comment 3 Jorge Villalobos [:jorgev] 2010-07-14 12:00:37 PDT
Yes, that's the one.
GUID: mozilla_cc@internetdownloadmanager.com
Comment 4 Wil Clouser [:clouserw] 2010-07-14 12:04:04 PDT
This was mentioned briefly in our meeting today, but that add-on isn't claiming compatibility with fx4.  If people are using it, they specifically overrode version checking and installed it.  It doesn't seem like we should get involved in a case like that - what if someone is testing/developing it and trying to get it working and we block it?  This is probably not the case for a top crasher, but it's still a valid scenario.  I guess the question is, do we really want to block add-ons when people have gone out of their way to install them?
Comment 5 Jorge Villalobos [:jorgev] 2010-07-15 15:57:21 PDT
All versions of the add-on are now limited to < 3.6.*, and the author has been notified about the problem. I'm OK with taking harsher measures in the future if there's no response from the author and the situation worsens, but I think for now we can WONTFIX.
Comment 6 Charles Jones 2010-07-16 02:46:58 PDT
Hi,

thanks for notifying about the problem!

It seems to be a problem with nsISelection interface. Note that the description of the interface clearly states that it’s FROZEN!
 
In former versions of Firefox the following code worked without a problem:
 
nsCOMPtr<nsISelection> selection;
rv = mWindow->GetSelection(getter_AddRefs(selection));
.................................................
 
PRUnichar* szText;
rv = selection->ToString(&szText);
if (NS_SUCCEEDED(rv))
{
    .....................
    using szText
    nsMemory::Free(szText);
}

But now selection->ToString(&szText) returns 0, that means  NS_SUCCEEDED(rv) true, BUT szText is not filled with data.

We have resolved the problem by adding the following changes:
 
PRUnichar* szText = NULL;
rv = selection->ToString(&szText);
if (NS_SUCCEEDED(rv) && szText)
{
    .....................
    using szText
    nsMemory::Free(szText);
}

please check nsISelection implementation and fix it if possible.
 
Regards


Charles Jones
Software engineer

Tonec Inc.
641 Lexington Avenue, 15th fl. 
New York, NY, 10022
Comment 7 christian 2010-07-16 09:00:13 PDT
We don't need to do anything about this for 3.6.x, right? The issue only affects 4.0?

Also, why did comment 5 block this on 3.6.x? Shouldn't it be anything < 3.7.x?
Comment 8 Jorge Villalobos [:jorgev] 2010-07-16 09:04:48 PDT
Compatibility for current versions is limited to a maxVersion of 3.6.*. I don't know if setting it to some variation of 3.7 would continue to cause crashes.
Comment 9 chris hofmann 2010-07-16 11:53:45 PDT
3.7 isn't the issue; not many users on those releases at all.  

4.0b1 has a quarter million users at this point and rising.  the crash reports from Internet Download Manager are obscuring other bugs that we need to be analyzing, and making a poor experience for users who don't understand the reason for their crashing problems and the connection to IDM, and might not remember they have extension compatibility turned off.

We need a way to solve this.  Blocking 4.0b1 (and b2 if needed) would raise attention to the problem among the users.  We could use the "soft blocking" override option so they could choice to use it if the wanted.

We could also use this a test case for e-mailing users, or redirecting them to SUMO information informing them of steps to reduce crashes they are encountering.  Is any of that stuff ready to roll out yet?
Comment 10 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2010-07-16 12:11:22 PDT
52% of yesterday's 4.0b1 Windows crashes came from users with Internet Download Manager installed (by addonid "mozilla_cc@internetdownloadmanager.com").
Comment 11 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2010-07-16 12:19:32 PDT
(In reply to comment #6)
> But now selection->ToString(&szText) returns 0, that means  NS_SUCCEEDED(rv)
> true, BUT szText is not filled with data.

Do you mean szText was assigned a null pointer or szText was not assigned any value at all?

I read through the implementation (nsTypedSelection::ToString), and I don't see any cases where it could possibly return a success rv without assigning to the out-param, and the only possibility that could lead to a success rv with a null out-param is out-of-memory.

In any case, the null-dereference that you describe could not be the cause of the myriad crashes that Internet Download Manager is causing in 4.0b1.
Comment 12 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2010-07-16 12:52:59 PDT
Also, most of the crashes are from users with 6.9.7 and 6.9.8, which are newer than the version 6.9.1 that's on AMO.
Comment 13 Jorge Villalobos [:jorgev] 2010-07-16 12:53:56 PDT
I just installed the IDM application on Windows, and this installs a different version of IDM CC. This is version 6.9.8 (on AMO it's 6.9.1), and it claims a maxVersion of Firefox 4.0. I think it's safe to say that the majority of users get the add-on installed this way. Furthermore, this extension uses its own update URL, so we have very little control in updating users.

It also looks like the extension installs itself in a way that it is impossible to uninstall from Firefox (at least when I tested on 3.6 on Windows 7).

Again, I move for blocklisting at least on 4.0b1.
Comment 14 Marcia Knous [:marcia - use ni] 2010-07-16 13:45:41 PDT
If you install it from their website, the only way to uninstall Version 6.9.8 from Firefox is to uninstall the program, make sure to check "Complete" and then let your computer reboot. When I do that it removes the extension from a trunk version of Firefox. But that doesn't guarantee all the cruft is removed. 

(In reply to comment #13)
> I just installed the IDM application on Windows, and this installs a different
> version of IDM CC. This is version 6.9.8 (on AMO it's 6.9.1), and it claims a
> maxVersion of Firefox 4.0. I think it's safe to say that the majority of users
> get the add-on installed this way. Furthermore, this extension uses its own
> update URL, so we have very little control in updating users.
> 
> It also looks like the extension installs itself in a way that it is impossible
> to uninstall from Firefox (at least when I tested on 3.6 on Windows 7).
> 
> Again, I move for blocklisting at least on 4.0b1.
Comment 15 Ryan Doherty (:rdoherty) 2010-07-16 14:46:49 PDT
(In reply to comment #13) 
> Again, I move for blocklisting at least on 4.0b1.

Is Charles Jones' info in comment #6 correct? Is this something Firefox will/needs to fix or IDM?

Can IDM update/downgrade/something itself instead of blocklisting?
Comment 16 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2010-07-16 15:02:05 PDT
comment 6 is not correct -- see comment 11 -- and even if it were, it wouldn't explain the crashes.
Comment 17 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2010-07-16 15:02:35 PDT
er, wouldn't explain the crashes that we're seeing.  It could explain some crashes if it were correct, though.
Comment 18 Ryan Doherty (:rdoherty) 2010-07-16 15:04:58 PDT
(In reply to comment #17)
> er, wouldn't explain the crashes that we're seeing.  It could explain some
> crashes if it were correct, though.

Ok, thanks. I want to make sure we exhaust all options before blocklisting, and it seems that's our best option right now.
Comment 19 Charles Jones 2010-07-16 16:07:32 PDT
(In reply to comment #11)
> Do you mean szText was assigned a null pointer or szText was not assigned any
> value at all?
it seems that no variable is assigned at all

> In any case, the null-dereference that you describe could not be the cause of
> the myriad crashes that Internet Download Manager is causing in 4.0b1.

we could repeat the crash on the first URL (http://vnexpress.net/GL/Home/) from the list from https://bugzilla.mozilla.org/show_bug.cgi?id=578281

first try to select a text in a column with blue links, then click on this link and we have the same crash in all cases. Once we have fixed the code as shown above, the crash did not appear anymore. It’s possible that users move mouse while clicking on links and this small mouse movement cause the text selection.
Comment 20 Benjamin Smedberg [:bsmedberg] 2010-07-19 07:38:55 PDT
Marking this a beta2 blocker which we'd like to get this week. Morgamic, let me know if that is a problem.
Comment 21 Shawn Wilsher :sdwilsh 2010-07-19 08:02:03 PDT
(In reply to comment #6)
> It seems to be a problem with nsISelection interface. Note that the description
> of the interface clearly states that it’s FROZEN!
Note that frozen interfaces can change in Firefox 4.0.  The Gecko version is 2.0, and all previously frozen interfaces are no longer frozen.
Comment 22 Charles Jones 2010-07-20 07:46:23 PDT
(In reply to comment #21)
> Note that frozen interfaces can change in Firefox 4.0.  The Gecko version is
> 2.0, and all previously frozen interfaces are no longer frozen.

1. Please give a link to Mozilla documentation where it’s explicitly documented. Besides I’ve read the following document:
https://developer.mozilla.org/en/XPCOM/XPCOM_changes_in_Gecko_1.9.3

And it says. It is possible for a binary component to be compatible with Mozilla 1.9.2 and Mozilla 2.0 by using the extra macro NS_IMPL_MOZILLA192_NSGETMODULE. See nsSampleModule.cpp for more details. 

And inside this sample:
http://mxr.mozilla.org/mozilla-central/source/xpcom/sample/nsSampleModule.cpp 

Look at the comment before this macro:
// The following line implements the one-and-only "NSGetModule" symbol 
// for compatibility with mozilla 1.9.2. You should only use this 
// if you need a binary which is backwards-compatible and if you use 
// interfaces carefully across multiple versions. 

2. Note that this bug also appears in Firefox 3.7, not only in Firefox 4.0

3. You did not change the CID for nsISelection, neither you did change its description. It’s a bug inside the interface implementation. The crashes are possible with any extension which call nsISelection::ToString(), also it’s possible with new extensions made with Gecko 2.0. Please check the interface implementation for nsISelection. If you can fix it, old versions of our extension (millions of customers) will not crash new Firefox anymore.

Please note that this crash is not our fault. There is no mention that ToString method may not assign return value even if NS_SUCCEEDED(rv) returns true. 
Look here:
http://www.mozilla.org/projects/embedding/embedapiref/embedapi74.html

It says:
ToString returns the whole selection into a plain text string. 

Syntax: 
wstring nsISelection::toString() 
Parameters: 
None.

nsresult: 
NS_OK if successful

Also I looked at idl and h files descriptions:

/**
 * Returns the whole selection into a plain text string.
*/
   wstring toString();

/**
  * Returns the whole selection into a plain text string.
*/
/* wstring toString (); */
NS_SCRIPTABLE NS_IMETHOD ToString(PRUnichar **_retval NS_OUTPARAM) = 0;

There is no mention here either.

Thus formally it's a bug of Firefox interface implementation

Regards

Charles
Tonec Inc.
Comment 23 Benjamin Smedberg [:bsmedberg] 2010-07-20 07:52:40 PDT
1) Yes, it is possible to write a single binary component which is compatible with Firefox 3.6 and Firefox 4. This will not be true in future releases. That doesn't mean that we haven't changed interfaces between Firefox 3.6 and 4 (we have).

2) There is no such thing as Firefox 3.7.

3) So what? The user still experiences a crash, no matter whose fault it is. You are welcome to file the bug in our implementation, preferably with a fix.
Comment 24 Michael Morgan [:morgamic] 2010-07-20 15:26:04 PDT
This is blocked for anything over 3.6.*.  For the blocklist page (http://www.mozilla.com/en-US/blocklist/) what do you guys want to say?
Comment 25 Jorge Villalobos [:jorgev] 2010-07-20 18:08:48 PDT
Reason: high crash volume in Firefox 4 pre-release versions.
Comment 26 Robert Strong [:rstrong] (use needinfo to contact me) 2010-07-20 20:23:08 PDT
From the discussion in this bug it isn't clear (at least to me) that IDM should have been blocklisted for 3.6.* vs 4.* and from several comments in bug 382356 it appears that people have been using it with 3.6 without any issues and are rather upset by it being blocklisted.
Comment 27 Robert Strong [:rstrong] (use needinfo to contact me) 2010-07-20 20:30:27 PDT
What I see in the blocklist
snip...
    <emItem id="mozilla_cc@internetdownloadmanager.com">
      <versionRange>
        <targetApplication id="{ec8030f7-c20a-464f-9b0e-13a3a9e97384}">
           <versionRange minVersion=" " maxVersion="3.7"/>
        </targetApplication>
      </versionRange>
      <versionRange minVersion="2.1" maxVersion="3.3">
        <targetApplication id="{ec8030f7-c20a-464f-9b0e-13a3a9e97384}">
           <versionRange minVersion="3.0a1" maxVersion="*"/>
        </targetApplication>
      </versionRange>
    </emItem>
Comment 28 Jorge Villalobos [:jorgev] 2010-07-20 20:36:44 PDT
The first versionRange looks wrong. There's no version range specified for IDM, so I assume it's applying to all, and the minVersion and maxVersion of Firefox seem to be backwards. It should be something like min = 3.7a1pre and max = 4.0.
Comment 29 Robert Strong [:rstrong] (use needinfo to contact me) 2010-07-20 20:38:40 PDT
exactly
Comment 30 Dave Townsend [:mossop] 2010-07-20 20:39:32 PDT
(In reply to comment #27)
> What I see in the blocklist
> snip...

I can confirm this. This means that all versions of IDM are blocked for all versions of Firefox up to 3.7 right now.
Comment 31 Jorge Villalobos [:jorgev] 2010-07-20 20:52:46 PDT
I reopened the IT bug that was filed to do this. Hopefully it will be dealt with soon.
Comment 32 Michael Morgan [:morgamic] 2010-07-20 21:44:39 PDT
Block was rolled back; I messed up the min/max.

Looks like it should be:
<versionRange minVersion="3.7a1pre" maxVersion="4.0"/>

Sorry folks.
Comment 33 Adrian 2010-07-20 21:50:36 PDT
So, then how do you get FF to unblock it?  Mine is still blocked.
Comment 34 Michael Morgan [:morgamic] 2010-07-20 21:58:52 PDT
It'll ping again and update the blocklist.xml within 24 hours but a quicker alternative is to update the blocklist.xml file manually to remove the invalid range.

Info on the blocklist file is here:
https://wiki.mozilla.org/Extension_Blocklisting:Testing#The_Blocklist_File

You'd want to remove the bad XML section, which is as follows:
      <versionRange>
        <targetApplication id="{ec8030f7-c20a-464f-9b0e-13a3a9e97384}">
           <versionRange minVersion=" " maxVersion="3.7"/>
        </targetApplication>
      </versionRange>
Comment 35 Robert Strong [:rstrong] (use needinfo to contact me) 2010-07-21 00:02:57 PDT
You can also do the following to make the blocklist service update immediately

Open Tools -> Error Console
In the code box paste the following line (all of it should be one line from Components to (null);.

Components.classes["@mozilla.org/extensions/blocklist;1"].getService(Components.interfaces.nsIBlocklistService).QueryInterface(Components.interfaces.nsITimerCallback).notify(null);

Press enter
Comment 36 tdba.316 2010-07-21 02:40:07 PDT
the latest version of IDM CC at
http://www.internetdownloadmanager.com/idmmzcc/4.02b/idmmzcc.xpi (6.9.9) fixed
the issues and compatibility problems with firefox 4.
Comment 37 Charles Jones 2010-07-21 02:46:02 PDT
I'm really tired to write to Firefox forums that

1. IDM CC 6.9.9 has a workaround and Firefox 3.7 and 4.0 DO NOT crash anymore with this version of extension
2. The reason of crash is FIREFOX BUG inside nsISeletion::ToString() implementation! Please FIX your bug instead of blacklisting!

Best regards,

Charles Jones
Software engineer

Tonec Inc.
641 Lexington Avenue, 15th fl. 
New York, NY, 10022
Comment 38 Reed Loden [:reed] (use needinfo?) 2010-07-21 03:09:51 PDT
(In reply to comment #37)
> 2. The reason of crash is FIREFOX BUG inside nsISeletion::ToString()
> implementation! Please FIX your bug instead of blacklisting!

As you were informed in comment #23, you should file a bug on the issue you're seeing. Commenting here doesn't get anything fixed if there is some type of bug in Gecko.
Comment 39 George 2010-07-21 08:34:41 PDT
(In reply to comment #32)
> Block was rolled back; I messed up the min/max.
> 
> Looks like it should be:
> <versionRange minVersion="3.7a1pre" maxVersion="4.0"/>
> 
> Sorry folks.

As admitted in comment 32 (and other comments), the blocking code was completely wrong. Obviously, it was not even tested by the developer.

Is it possible that all future blocklists will have to be tested by the developer as well as a test team to ensure that such an obvious bug doesn't affect a lot of users again?
Comment 40 Charles Jones 2010-07-21 10:20:07 PDT
(In reply to comment #38)
> As you were informed in comment #23, you should file a bug on the issue you're
> seeing. Commenting here doesn't get anything fixed if there is some type of  > bug in Gecko.

I would be glad to file a bug, but since I'm not from Mozilla development team, I'm not sure how it should be done correctly. Any help here would be appreciated.

Regards,
Charles Jones

Tonec Inc.
IDM Development Team
Comment 41 Benjamin Smedberg [:bsmedberg] 2010-07-21 10:21:51 PDT
https://developer.mozilla.org/en/Bug_writing_guidelines
Comment 42 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2010-07-21 10:34:46 PDT
My guess as to why you're seeing the behavior you describe with nsISelection is that the nsISelection interface has changed; there's a new method that was added, and the interface is no longer frozen.  So if you're assuming the nsISelection interface is still as it was in older versions, then you'd be calling the method:
    void selectionLanguageChange(in boolean langRTL);
instead of:
    wstring toString();
so the variable you're expecting to be a PRUnichar** is actually a PRBool.

If that's the cause of your problem, this is not a Gecko bug; interfaces have been unfrozen for Gecko 2.0.
Comment 43 Michael Morgan [:morgamic] 2010-07-21 12:57:39 PDT
Created attachment 459118 [details]
updated blocklist.xml

Alright gents - can you take a peek at this one to make sure it's right?  Thanks.
Comment 44 Robert Strong [:rstrong] (use needinfo to contact me) 2010-07-21 13:06:21 PDT
Comment on attachment 459118 [details]
updated blocklist.xml

><?xml version="1.0" encoding="UTF-8"?>
><blocklist xmlns="http://www.mozilla.org/2006/addons-blocklist">
>  <emItems>
>    <emItem id="fdm_ffext@freedownloadmanager.org">
>      <versionRange minVersion="1.0" maxVersion="1.3.1">
>        <targetApplication id="{ec8030f7-c20a-464f-9b0e-13a3a9e97384}">
>           <versionRange minVersion="3.0a1" maxVersion="*"/>
>        </targetApplication>
>      </versionRange>
>    </emItem>
>    <emItem id="langpack-vi-VN@firefox.mozilla.org">
>      <versionRange minVersion="2.0" maxVersion="2.0"/>
>    </emItem>
>    <emItem id="masterfiler@gmail.com">
>      <versionRange severity="3"/>
>    </emItem>
>    <emItem id="mozilla_cc@internetdownloadmanager.com">
>      <versionRange>
>        <targetApplication id="{ec8030f7-c20a-464f-9b0e-13a3a9e97384}">
>           <versionRange minVersion="3.7a1pre" maxVersion="4.0"/>
>        </targetApplication>
>      </versionRange>
The author claims that version 6.9.9 works with 3.7 through 4.0 so I don't think we should blocklist 6.9.9 and above of the add-on unless there is reason to believe that 6.9.9 causes a crash.
Comment 45 Robert Strong [:rstrong] (use needinfo to contact me) 2010-07-21 13:09:52 PDT
Comment on attachment 459118 [details]
updated blocklist.xml

Minusing per comment #44. I'll be glad to verify everything works properly across 3.6 and trunk when you resubmit with the changes requested.
Comment 46 Robert Strong [:rstrong] (use needinfo to contact me) 2010-07-21 13:11:03 PDT
Note that I don't have 6.9.9 of IDM so I won't be testing the functionality of the add-on... just the blocklist
Comment 47 Charles Jones 2010-07-21 15:30:07 PDT
(In reply to comment #42)
> My guess as to why you're seeing the behavior you describe with nsISelection is
> that the nsISelection interface has changed; there's a new method that was
> added, and the interface is no longer frozen.  So if you're assuming the
> nsISelection interface is still as it was in older versions, then you'd be
> calling the method:
>     void selectionLanguageChange(in boolean langRTL);
> instead of:
>     wstring toString();
> so the variable you're expecting to be a PRUnichar** is actually a PRBool.
> If that's the cause of your problem, this is not a Gecko bug; interfaces have
> been unfrozen for Gecko 2.0.

David, do you program interface implementations often? nsISelection in Gecko 2 did not change. Mozilla developers created new nsISelection2 interface which is a successor of nsISelection. It can be seen in idl file: interface nsISelection2 : nsISelection. It means that methods of basic class have not changed and only new methods have been added. nsISelection still remains FROZEN, and QueryInterface can accept old nsISelection CID (B2C7ED59-8634-4352-9E37-5484C8B6E4E1) and new nsISelection2 CID (5d21d5fe-3691-4716-a334-4691eea54d29) and will return the pointer to the same object. If we do not need new methods, we can get a pointer for Selection object using old CID. And we use it that way. And all old methods will work flawlessly. I do not have to write here the theoretical footing of programming, do I?

If developers wanted to get rid of old nsISelection, they would change CID or make interface nsISelection2 : nsISupports and add all old methods over again. After that QueryInterface with old nsISelection CID will return that it does not know such interface. 

But I think that someone at the beginning had a good idea and frozen interfaces still remain frozen and you should come to know the particulars of it a bit further.
Comment 48 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2010-07-21 15:36:41 PDT
The nsISelection IID did change; I don't see how CIDs are relevant here.
Comment 49 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2010-07-21 15:39:52 PDT
... but you're right that the nsISelection2 IID should also have changed, and it didn't.
Comment 50 Charles Jones 2010-07-22 12:24:40 PDT
I apologize I had a strange version of Gecko SDK 2.0. I re-downloaded it and found that nsISelection and its CID (or IID) had changed indeed.

The things seem to be worse. There is no protection when former FROZEN interfaces are returned as pointers in parameters of functions.

In our case when we compile with old Gecko SDK, the following code:

nsCOMPtr<nsISelection> selection;
rv = mWindow->GetSelection(getter_AddRefs(selection));

it returns rv NS_OK, and selection is not NULL. It seems to be OK and we use it
if (NS_SUCCEEDED(rv) && selection)
   ..................
And some methods do even work correctly, but ToString() introduced a problem
 
I may add another checking like
nsCOMPtr<nsISelection> selection2 = do_QueryInterface(selection, &rv);
if (NS_SUCCEEDED(rv) && selection2)
   ....................

But it's not a built in protection
 
The problem is that pointers to former FROZEN interfaces are returned the same way in many cases. We can do additional checking, but other extension developers may not do it.
 
Maybe in case of such global changes, you should not support old versions like it's described here
https://developer.mozilla.org/en/XPCOM/XPCOM_changes_in_Gecko_1.9.3
"It is possible for a binary component to be compatible with Mozilla 1.9.2 and Mozilla 2.0 by using the extra macro NS_IMPL_MOZILLA192_NSGETMODULE. See nsSampleModule.cpp for more details."
 
If somebody compiles his extension with SDK 1.9 he will have old interface descriptions, and such crashes are possible with new versions of Firefox.
 
If somebody compiles his extension with SDK 2.0, he may have same kind of crashes with Firefox 3.6 and below. The compiler won't show any errors, and the extension may work correctly on first look, and it seemed that only component registration should be changed for the extension to work with Firefox 4.0b2pre. But the problems like I described above may come up later.
 
Having analyzed that I think that it's hardly possible to change nsISelection implementation so that our old extension won't crash. On the other hand, old extensions are not registered in Firefox 4.0b2pre and will not be loaded. Also they won't be loaded in the final release of Firefox 4. Thus this topic should be closed.

Do you plan to change FROZEN interfaces in the future? Or there will be no changes in FROZEN interface except those made in SDK 2.0pre?
Comment 51 Michael Morgan [:morgamic] 2010-07-23 16:46:52 PDT
Created attachment 459963 [details]
v2, adding 6.9.9 restriction
Comment 52 Robert Strong [:rstrong] (use needinfo to contact me) 2010-07-23 17:37:39 PDT
Comment on attachment 459963 [details]
v2, adding 6.9.9 restriction

><?xml version="1.0" encoding="UTF-8"?>
><blocklist xmlns="http://www.mozilla.org/2006/addons-blocklist">
>  <emItems>
>    <emItem id="fdm_ffext@freedownloadmanager.org">
>      <versionRange minVersion="1.0" maxVersion="1.3.1">
>        <targetApplication id="{ec8030f7-c20a-464f-9b0e-13a3a9e97384}">
>           <versionRange minVersion="3.0a1" maxVersion="*"/>
>        </targetApplication>
>      </versionRange>
>    </emItem>
>    <emItem id="langpack-vi-VN@firefox.mozilla.org">
>      <versionRange minVersion="2.0" maxVersion="2.0"/>
>    </emItem>
>    <emItem id="masterfiler@gmail.com">
>      <versionRange severity="3"/>
>    </emItem>
>    <emItem id="mozilla_cc@internetdownloadmanager.com">
>      <versionRange minVersion="2.1" maxVersion="3.3"/>
>      <versionRange minVersion="*" maxVersion="6.9.9">
6.9.9 is the one we don't want to block on trunk. :(

>        <targetApplication id="{ec8030f7-c20a-464f-9b0e-13a3a9e97384}">
>           <versionRange minVersion="3.7a1pre" maxVersion="4.0"/>
>        </targetApplication>
>      </versionRange>
You also removed the following
      <versionRange minVersion="2.1" maxVersion="3.3">
        <targetApplication id="{ec8030f7-c20a-464f-9b0e-13a3a9e97384}">
           <versionRange minVersion="3.0a1" maxVersion="*"/>
        </targetApplication>
      </versionRange>

>    </emItem>
Comment 53 Robert Strong [:rstrong] (use needinfo to contact me) 2010-07-23 17:47:07 PDT
(In reply to comment #52)
> Comment on attachment 459963 [details]
> v2, adding 6.9.9 restriction
> 
> ><?xml version="1.0" encoding="UTF-8"?>
> ><blocklist xmlns="http://www.mozilla.org/2006/addons-blocklist">
> >  <emItems>
> >    <emItem id="fdm_ffext@freedownloadmanager.org">
> >      <versionRange minVersion="1.0" maxVersion="1.3.1">
> >        <targetApplication id="{ec8030f7-c20a-464f-9b0e-13a3a9e97384}">
> >           <versionRange minVersion="3.0a1" maxVersion="*"/>
> >        </targetApplication>
> >      </versionRange>
> >    </emItem>
> >    <emItem id="langpack-vi-VN@firefox.mozilla.org">
> >      <versionRange minVersion="2.0" maxVersion="2.0"/>
> >    </emItem>
> >    <emItem id="masterfiler@gmail.com">
> >      <versionRange severity="3"/>
> >    </emItem>
> >    <emItem id="mozilla_cc@internetdownloadmanager.com">
> >      <versionRange minVersion="2.1" maxVersion="3.3"/>
> >      <versionRange minVersion="*" maxVersion="6.9.9">
> 6.9.9 is the one we don't want to block on trunk. :(
> 
> >        <targetApplication id="{ec8030f7-c20a-464f-9b0e-13a3a9e97384}">
> >           <versionRange minVersion="3.7a1pre" maxVersion="4.0"/>
> >        </targetApplication>
> >      </versionRange>
> You also removed the following
>       <versionRange minVersion="2.1" maxVersion="3.3">
>         <targetApplication id="{ec8030f7-c20a-464f-9b0e-13a3a9e97384}">
>            <versionRange minVersion="3.0a1" maxVersion="*"/>
>         </targetApplication>
>       </versionRange>
> 
> >    </emItem>
bah... I see you changed it instead. wish this were a diff. :)

So, you are blocking all version of Firefox for IDM 2.1 to 3.3. Personally I would prefer leaving it the way it was.

and 6.9.9 should be the first version of IDM that is compatible on 3.7a1pre and above. I'd go with minVersion="3.7a1pre" maxVersion="*" for the targetApp range
Comment 54 Mike Beltzner [:beltzner, not reading bugmail] 2010-08-04 13:17:22 PDT
poke/prod now that you're back from blackhat
Comment 55 Michael Morgan [:morgamic] 2010-08-04 13:36:00 PDT
Created attachment 462895 [details] [diff] [review]
v3, adding * for app range and changing item version to 6.9.8
Comment 56 Robert Strong [:rstrong] (use needinfo to contact me) 2010-08-04 13:46:53 PDT
Comment on attachment 462895 [details] [diff] [review]
v3, adding * for app range and changing item version to 6.9.8

>    <emItem id="mozilla_cc@internetdownloadmanager.com">
>      <versionRange minVersion="2.1" maxVersion="3.3"/>
As noted previously... why are you changing the following?
      <versionRange minVersion="2.1" maxVersion="3.3">
        <targetApplication id="{ec8030f7-c20a-464f-9b0e-13a3a9e97384}">
           <versionRange minVersion="3.0a1" maxVersion="*"/>
        </targetApplication>
      </versionRange>
This prevents older versions of idm from working on versions of Firefox.


>      <versionRange minVersion="*" maxVersion="6.9.8">
Shouldn't matter for this instance but you could use 6.9.8* to get everything prior to 6.9.9

I'm off to the dentist shortly and don't have time to manually test.

r=me with the first item put back the way it was or an explanation for the change.
Comment 57 Michael Morgan [:morgamic] 2010-08-04 13:49:39 PDT
The other line is there from another block described in bug 382356.

See also:
http://www.mozilla.com/en-US/blocklist/

(this is also what caused all the traffic to bug 382356 a couple of weeks ago)
Comment 58 Robert Strong [:rstrong] (use needinfo to contact me) 2010-08-04 13:57:42 PDT
It was originally as follows
      <versionRange minVersion="2.1" maxVersion="3.3">
        <targetApplication id="{ec8030f7-c20a-464f-9b0e-13a3a9e97384}">
           <versionRange minVersion="3.0a1" maxVersion="*"/>
        </targetApplication>
      </versionRange>


You then added a malformed entry as follows along with that entry
       <versionRange>
         <targetApplication id="{ec8030f7-c20a-464f-9b0e-13a3a9e97384}">
            <versionRange minVersion=" " maxVersion="3.7"/>
         </targetApplication>
       </versionRange>

It was the malformed entry that caused the traffic to bug 382356 a couple of weeks ago but the original entry was fine.

Do you think the blocklist won't properly handle 
    <emItem id="mozilla_cc@internetdownloadmanager.com">
      <versionRange minVersion="2.1" maxVersion="3.3">
        <targetApplication id="{ec8030f7-c20a-464f-9b0e-13a3a9e97384}">
           <versionRange minVersion="3.0a1" maxVersion="*"/>
        </targetApplication>
      </versionRange>
      <versionRange minVersion="*" maxVersion="6.9.8">
        <targetApplication id="{ec8030f7-c20a-464f-9b0e-13a3a9e97384}">
           <versionRange minVersion="3.7a1pre" maxVersion="*"/>
        </targetApplication>
      </versionRange>
    </emItem>

and will properly handle
    <emItem id="mozilla_cc@internetdownloadmanager.com">
      <versionRange minVersion="2.1" maxVersion="3.3"/>
      <versionRange minVersion="*" maxVersion="6.9.8">
        <targetApplication id="{ec8030f7-c20a-464f-9b0e-13a3a9e97384}">
           <versionRange minVersion="3.7a1pre" maxVersion="*"/>
        </targetApplication>
      </versionRange>
    </emItem>
Comment 59 Michael Morgan [:morgamic] 2010-08-04 14:20:24 PDT
People saw the blocklist page then went to that bug is what I'm saying.  I know what caused the situation, which is why I am not a fan of changing old entries if we don't have to.

But yeah, we can edit the old entry too if that's what you're asking for.
Comment 60 Robert Strong [:rstrong] (use needinfo to contact me) 2010-08-04 14:26:42 PDT
(In reply to comment #59)
> People saw the blocklist page then went to that bug is what I'm saying.  I know
> what caused the situation, which is why I am not a fan of changing old entries
> if we don't have to.
> 
> But yeah, we can edit the old entry too if that's what you're asking for.
Not at all. You changed the original entry from:
      <versionRange minVersion="2.1" maxVersion="3.3">
        <targetApplication id="{ec8030f7-c20a-464f-9b0e-13a3a9e97384}">
           <versionRange minVersion="3.0a1" maxVersion="*"/>
        </targetApplication>
      </versionRange>
To:
      <versionRange minVersion="2.1" maxVersion="3.3"/>

I'm not a fan of changing existing entries either which is why I keep harping on this.

The original entry blocklists IDM 2.1 through 3.3 for Firefox 3.0a1 through all greater versions.

You changed that to blocklist IDM 2.1 through 3.3 for all versions of Firefox.
Comment 61 Michael Morgan [:morgamic] 2010-08-04 14:27:07 PDT
Created attachment 462923 [details] [diff] [review]
v4, changes old entry as well...
Comment 62 Michael Morgan [:morgamic] 2010-08-04 14:29:13 PDT
Rob - that might have been IT when they backed it out, but either way I'm fine w/ v4.
Comment 63 Robert Strong [:rstrong] (use needinfo to contact me) 2010-08-04 14:32:01 PDT
Quite possibly. I verified that this was in the previous blocklist at one time and that it even match with the blocklist page. Specifically, "v2.1-3.3 for Firefox 3.0a1 and newer".

I need to run an will hopefully be able to review later tonight.
Comment 64 Michael Morgan [:morgamic] 2010-08-04 14:33:26 PDT
Alright - thanks for catching that.  It is indeed from the IT rollback.  Prod right now doesn't have the targetApp range for 2.1-3.3.
Comment 65 Robert Strong [:rstrong] (use needinfo to contact me) 2010-08-05 01:18:02 PDT
Comment on attachment 462923 [details] [diff] [review]
v4, changes old entry as well...

Glad I manually tested this. :)

>    <emItem id="mozilla_cc@internetdownloadmanager.com">
>      <versionRange minVersion="*" maxVersion="6.9.8">
Per https://wiki.mozilla.org/Extension_Blocklisting:Code_Design
minVersion can't use the * wildcard since minVersion should always be a specific version and just * will always be a version number greater than the maxVersion.

Change minVersion to 0

>    <emItem id="support@daemon-tools.cc">
>      <versionRange minVersion=" " maxVersion="1.0.0.5"/>
>    </emItem>
Please change this and other minVersion's that are currently " " to 0. This works since text is evaluated as a version but it really should be changed to 0.

r=me with that
Comment 66 Robert Strong [:rstrong] (use needinfo to contact me) 2010-08-10 00:54:20 PDT
btw: I don't have a problem with landing this with minVersion=" " and maxVersion="6.9.8" and fix the minVersion=" " in bug 584846 later.
Comment 67 Mike Beltzner [:beltzner, not reading bugmail] 2010-08-10 13:42:54 PDT
What do I need to do to get this done?
Comment 68 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2010-08-10 16:26:00 PDT
(In reply to comment #10)
> 52% of yesterday's 4.0b1 Windows crashes came from users with Internet Download
> Manager installed (by addonid "mozilla_cc@internetdownloadmanager.com").

For reference, from yesterday's data, 55% of 4.0b1 crashes came from users with IDM installed, but only 8% of 4.0b2 crashes.  (Is that because the XPCOM registration changes make the extension not do anything?  Or is 8% still high?)

beta3 has nsISelection reverted back to its old state.
Comment 69 Michael Morgan [:morgamic] 2010-08-11 01:42:00 PDT
Ranges are updated server-side:
    <emItem id="mozilla_cc@internetdownloadmanager.com">
      <versionRange minVersion=" " maxVersion="6.9.8">
        <targetApplication id="{ec8030f7-c20a-464f-9b0e-13a3a9e97384}">
           <versionRange minVersion="3.7a1pre" maxVersion="*"/>
        </targetApplication>
      </versionRange>
      <versionRange minVersion="2.1" maxVersion="3.3">
        <targetApplication id="{ec8030f7-c20a-464f-9b0e-13a3a9e97384}">
           <versionRange minVersion="3.0a1" maxVersion="*"/>
        </targetApplication>
      </versionRange>
    </emItem>

Blocklist page updated and will be live shortly:
http://www.mozilla.com/en-US/blocklist/
Comment 70 Shawn Wilsher :sdwilsh 2010-08-30 16:41:51 PDT
Note that we are still having issues with IDM CC on nightlies.  Looks like 7.1.1 and 7.1.2 are also problematic (bug 592125)
Comment 71 Benjamin Smedberg [:bsmedberg] 2010-08-30 16:50:29 PDT
*If* this incompatibility comes from the nsIChannel change, then it won't be an issue. Really hard to tell, though!
Comment 72 Zied 2011-03-25 07:43:09 PDT
I just want to unblock this addon, how can i do this??
Comment 73 Matthias Versen [:Matti] 2011-03-25 07:49:03 PDT
>I just want to unblock this addon, how can i do this??
Download the most recent version of this extension. This blocked only older versions
Comment 74 Vo Viet Anh 2011-04-03 00:29:30 PDT
Please! Let me use IDM at my own risk, no matter what. I can... I can sign a pact!
Comment 75 Muhammad Yousuf Saad 2011-05-30 12:40:49 PDT
please help me to download videos from youtube from monzila firefox browser, it has been blocked due to security reasons.
Comment 76 d 2011-05-30 13:11:30 PDT
People, please do not post in this bug any more, it's been resolved.

Muhammad, try https://addons.mozilla.org/firefox/addon/easy-youtube-video-downl-10137
Comment 77 jamdar 2012-03-14 13:22:45 PDT
Created attachment 605921 [details] [diff] [review]
i need idm to use in this browser
Comment 78 Merry Kristianty 2012-05-15 06:19:09 PDT
please don't blocked it, i need it. . please please please please please
Comment 79 Merry Kristianty 2012-05-15 06:22:06 PDT
please don't block it. . pleaseeeeeeeeee. . . . . . . . i hope you understand. . . .
Comment 80 Merry Kristianty 2012-05-15 06:23:49 PDT
pleaseeeee understanddddddd. . . . . . . .  .
Comment 81 Andre Klapper 2012-05-15 06:36:11 PDT
STOP ADDING THESE USELESS COMMENTS!
THEY CREATE BUGMAIL FOR EVERYBODY HERE!
Comment 82 Nazir 2012-07-14 09:44:20 PDT
I would like to tell Mozilla to stop fooling people by saying that IDM is responsible for any crashes...The truth is that Mozilla has received report from everywhere to block IDM because the latter permits us to download audio/video directly and IDM remains the best...
Comment 83 Jorge Villalobos [:jorgev] 2013-06-13 16:52:50 PDT
*** Bug 530524 has been marked as a duplicate of this bug. ***
Comment 84 kohinoorbatterymlg 2015-04-08 05:45:28 PDT Comment hidden (spam)

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