303075 - tabbrowser.xml : getBrowser().removeProgressListener just resets the progressListener to null

Status: RESOLVED FIXED

(Firefox :: Tabbed Browser, defect)

Description

[Jochen]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b4) Gecko/20050802 Firefox/1.0+
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b4) Gecko/20050802 Firefox/1.0+

Trunk only. I don't know if this is covered by bug 297155 ...

<method name="removeProgressListener">
  ...
  for (var i = 0; i < this.mProgressListeners.length; i++) {
       if (this.mProgressListeners[i] == aListener) {
         this.mProgressListeners.[i] = null;
         break;
       }
  }
  ...

this will set the progress listener to null and
will break '<method name="setIcon">', because there is no 'onLinkIconAvailable'
in 'p'. 
(error message in JavaScript Console)

<method name="removeProgressListener">
 ...
  this.mProgressListeners.splice(i, 1);
  break;
 ...              
</method>

fixes the problems for me. (just for testing)

So, adding a progressListener and removing it will force
the throbber to never stop, because 
'onLinkIconAvailable' in 'p' (which is null) is not present.

 



Reproducible: Always



Expected Results:  
Remove everything from "mProgressListeners"

Comment 1

[Jochen]

Damn! This is a bad bug report!

I just wanted to say, that adding and removing a progressListener (e.g. from an
extension)
will cause a 'null' in 'mProgressListeners', leading to never ending 'throbbers'
and 'statusbar-progressmeters'.

Comment 2

[Jochen]

Attachment: Testcase

Comment 3

[Aaron Slunt]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b4) Gecko/20050802
Firefox/1.0+ ID:2005080207

WFM. I allow the js to run, but everything still loads a-ok for me.

Reporter, try reproducing with a clean profile.

Comment 4

[O. Atsushi (Torisugari)]

(In reply to comment #3)
> WFM. I allow the js to run, but everything still loads a-ok for me.
> 
> Reporter, try reproducing with a clean profile.

You need to save the testcase in the local disk.

Comment 5

[Aaron Slunt]

Doesn't matter if I save it to my computer or not, I still get the same warnings
and I give access to what I'm being warned about, and still no problems for me.

Comment 6

[Aaron Slunt]

Sorry here, my js console said nothing, but after browsing for 5 minutes I
finally took notice to the hiccups. Sorry. This is confirmed

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b4) Gecko/20050802
Firefox/1.0+ ID:2005080207

Comment 7

[Jochen]

(In reply to comment #4)
> 
> You need to save the testcase in the local disk.
> 
Sorry. I forgot the "saving to local disk".

Comment 8

[Aaron Slunt]

Attachment: patch

This patch reflects comment 0. The reporter is correct: using splice() is the
proper way to remove array elements. Else, the null entry will still be in the
array, and then this mess is caused. This has been tested, and it works.

Comment 9

[Asa Dotzler [:asa]]

mconnor, can you shephard this in?

Comment 10

[:Gavin Sharp [email: [e-mail]]]

Checking in tabbrowser.xml;
/cvsroot/mozilla/toolkit/content/widgets/tabbrowser.xml,v  <--  tabbrowser.xml
new revision: 1.98; previous revision: 1.97
done