Open
Bug 320663
Opened 19 years ago
Updated 2 years ago
addProgressListener from Tab Browser does not always honor filter flags
Categories
(Toolkit :: UI Widgets, defect)
Toolkit
UI Widgets
Tracking
()
UNCONFIRMED
People
(Reporter: mark, Unassigned)
Details
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8) Gecko/20051111 Firefox/1.5 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8) Gecko/20051111 Firefox/1.5 Add a Progress Listener with filter flags like this: gBrowser.addProgressListener(browserProgressListener, Components.interfaces.nsIWebProgress.NOTIFY_STATE_ALL | Components.interfaces.nsIWebProgress.NOTIFY_LOCATION); Then in a browser with multiple tabs, click on a new tab. The onSecurityChange handler is called (incorrectly). (If you do not have an onSecurityChange handler defined, the dispatcher will fail silently, and all other progress events are lost.) The calls appear to be in toolkit/content/widgets/tabbrowser.xml, e.g: 682 for (i = 0; i < this.mProgressListeners.length; i++) { 683 p = this.mProgressListeners[i]; 684 if (p) { 685 p.onLocationChange(webProgress, null, loc); 686 if (securityUI) 687 p.onSecurityChange(webProgress, null, securityUI.state); The filter flags are ignored. [Also, shouldn't that loop work "backwards" in case listeners are removed by the handlers?] The user-visible manifestation of this was that when my extension (which had no dummy handler for onSecurityChange) was installed, the browser title bar failed to update and fastFind worked only in the first tab. This is because those are set up after line 687 above. Reproducible: Always Expected Results: Making the filters work according to spececification will probably break existing extensions. Fixing the event dispatch loop to check if the handler exists would avoid some difficult-to-find bugs. This was done for onLinkIconAvailable. At a minimum, this should be documented.
Comment 1•19 years ago
|
||
also, that function should probably use try..catch so that exceptions from one handler don't affect others.
Comment 2•19 years ago
|
||
Well, you're probably supposed to implement nsIWebProgressListener fully. onLinkIconAvailable is not a part of the interface, so that's why it's checked for. (That is not to say that the current code is all correct and clear.)
Component: General → XUL Widgets
Product: Firefox → Toolkit
QA Contact: general → xul.widgets
Version: unspecified → Trunk
Comment 3•18 years ago
|
||
As a workaround, you can just implement unused handlers as follows: onSecurityChange: function () { throw Components.results.NS_ERROR_NOT_IMPLEMENTED; } This shouldn't harm program flow but continues to remind you of the unfixed bug.
Comment 4•18 years ago
|
||
(In reply to comment #3) > onSecurityChange: function () { > throw Components.results.NS_ERROR_NOT_IMPLEMENTED; > } Although this would be the best solution, it sometimes seems to lead to problems with an uncaught internal exception being returned at some point. A less risky way is to simply silently back out using something along the lines of: onSecurityChange: function () { return; } HTH.
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•