433748 - Error: this._processor is null

Status: VERIFIED FIXED

(Firefox Graveyard :: RSS Discovery and Preview, defect)

Description

[Carsten Book [:Tomcat]]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9) Gecko/2008051206 Firefox/3.0

Steps to reproduce:
-> Go to http://www.syndic8.com/
-> Click on the "5 random syndicated feeds:" XML Button
-> Subscribe to this via Live Bookmarks
--> Check the Error Console

Error: this._processor is null
Source File: file:///C:/Program%20Files/Mozilla%20Firefox/components/nsLivemarkService.js
Line: 934

Comment 2

[Lucas Malor (mail: c6kfnkn2uc AT snkmail DOT c0m)]

(In reply to comment #0)
> Steps to reproduce:
> -> Go to http://www.syndic8.com/
> -> Click on the "5 random syndicated feeds:" XML Button
> -> Subscribe to this via Live Bookmarks
> --> Check the Error Console
> 

It works for me.
Can you install the latest trunk ( http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-trunk/ ) and test this bug in safe mode, or better with a blank profile? Disable also all plugins temporary.

If it works in this way, but not with your normal profile, probably it's a problem caused by an extension. So try to disable them one-by-one.

When you find the problematic extension(s), inform us and its author, and change the resolution of this bug to invalid.

Comment 3

[pd]

This bug is annoying the buggery out of me. I'm a web designer and every minute , if not more frequently, Firebug is displaying this as an error. That means I have to repeatedly open Firebug and double check that's it's not an error with my code.

Extremely frustrating. Enough to boost the status above "normal" IMHO.

Can this please be fixed in the next security and stability update? I don't want to have to go alpha on my main development machine just to solve this issue since I need a reliable and predictable environment with to develop.

Comment 5

[Vladimir Tkach]

Simple filter for this notice (fix for nsLivemarkService.js)
 /**
   * See nsIStreamListener.idl
   */
  onDataAvailable: function LLL_onDataAvailable(aRequest, aContext, aInputStream, aSourceOffset, aCount) {
    if (this._processor != null)
       this._processor.onDataAvailable(aRequest, aContext, aInputStream,
                                       aSourceOffset, aCount);
  },
  /**
   * See nsIRequestObserver.idl
   */

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Vladimir, would this fix the problem and populate the feed folder?

Comment 8

[Phil Ringnalda (:philor)]

Bad job of morphing. This bug is about the "Error: this._processor is null" error, which happens when data is available after the processor has been nulled out. It's not about showing some items from not-well-formed feeds, or trying to turn malformed XML into something that can be parsed.

Comment 9

[(no longer active)]

So, is this a dupe of bug 388275?  The patch from that bug adds the necessary null checks to onDataAvailable <http://hg.mozilla.org/mozilla-central/rev/7ebd8c4e8315>.

Comment 10

[Phil Ringnalda (:philor)]

That fixed FeedConverter.js so previewing a malformed feed doesn't throw, but not nsLivemarkService.js, so adding a malformed feed as a livemark (or, I presume, having one which later becomes malformed) still throws.

Comment 11

[(no longer active)]

Attachment: Patch (v1)

OK, obvious patch.

Comment 12

[Dietrich Ayala (:dietrich)]

Comment on attachment 374688 [details] [diff] [review]
Patch (v1)

>@@ -667,17 +668,18 @@ LivemarkLoadListener.prototype = {
>       this._setResourceTTL(ERROR_EXPIRATION);
>       this._isAborted = true;
>       MarkLivemarkLoadFailed(this._livemark.folderId);
>       this._livemark.locked = false;
>       return;
>     }
>     // Set an expiration on the livemark, for reloading the data
>     try {
>-      this._processor.onStopRequest(aRequest, aContext, aStatus);
>+      if (this._processor)
>+        this._processor.onStopRequest(aRequest, aContext, aStatus);
> 

trying to clarify what's going on here: onStopRequest is called after handleResult, with possible onDataAvailable calls in between?

Comment 13

[(no longer active)]

(In reply to comment #12)
> trying to clarify what's going on here: onStopRequest is called after
> handleResult, with possible onDataAvailable calls in between?

Yes, that should be the case.

Comment 14

[Dietrich Ayala (:dietrich)]

(In reply to comment #13)
> (In reply to comment #12)
> > trying to clarify what's going on here: onStopRequest is called after
> > handleResult, with possible onDataAvailable calls in between?
> 
> Yes, that should be the case.

what data is available after the processor decides that processing is complete?

(i just want to make sure this patch is not just a band-aid for a problem that exists somewhere else...)

Comment 15

[(no longer active)]

(In reply to comment #14)
> what data is available after the processor decides that processing is complete?

I'm not really sure what causes this behavior.

> (i just want to make sure this patch is not just a band-aid for a problem that
> exists somewhere else...)

I don't know this code well, so yes this could in fact be a band-aid for another problem, I'm afraid.

Comment 16

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #8)
> Bad job of morphing. This bug is about the "Error: this._processor is null"
> error, which happens when data is available after the processor has been nulled
> out. It's not about showing some items from not-well-formed feeds, or trying to
> turn malformed XML into something that can be parsed.

I've never talked about an not-well-formed feed. My comment is based on the same feed Carsten mentioned in comment 0. It is well-formed but causes a Livebookmark not to fill up with it's content. Do the steps from comment 0 and see what is happening. You will see that I haven't morphed anything! The current summary doesn't tell anything if you wanna query for a specific issue.

Comment 17

[Phil Ringnalda (:philor)]

Whether or not you knew that you were talking about a not-well-formed feed, at 16:30 on the 24th you *were* talking about a not-well-formed feed. At that time, one of the random titles had an unescaped HTML entity in it. Because the preview page code chose to display the items it gets before the parser hits a fatal error, it showed some items; because the livemark code chose to not display any items when the parser hits a fatal error, it did not show any items. The summary reflects *exactly* what this specific issue is about; if you want a bug about livemarks deciding to show the items parsed before a fatal error, you need a new bug.

Comment 18

[Phil Ringnalda (:philor)]

sayrer's more likely to actually know, but I think it's wallpaper, but necessary wallpaper. I think what's happening is that you have an HTTP channel where you called asyncOpen, which is sending you onDataAvailable calls as it loads the file, and you pass that data to the parser, and the parser hits a fatal error and tells you about it, so you give up on parsing, but don't call cancel() on the loadgroup, so you keep getting onDataAvailable calls for the rest of the feed. However, if I add a |this._livemark.loadGroup.cancel(Components.results.NS_BINDING_ABORTED)| in before nulling out the _processor, I still get the same this._processor is null error, so I'd guess you can't actually say "cancel right this instant, and don't you dare give me any more data."

Comment 19

[Dietrich Ayala (:dietrich)]

Comment on attachment 374688 [details] [diff] [review]
Patch (v1)

r=me, thanks for the patch. and thanks for the analysis philor.

Comment 20

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/070bce07a827

Comment 21

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

Should we return from onStopRequest early if _processor is null?

Comment 22

[(no longer active)]

(In reply to comment #21)
> Should we return from onStopRequest early if _processor is null?

I'm not sure.  The rest of the work performed by onStopRequest does not depend on _processor, but whether we should be doing that work is beyond me.  Dietrich?

Comment 23

[Phil Ringnalda (:philor)]

Being not-well-formed isn't an invitation to ignore caching headers and check constantly, so the only way you would want to return early would be if you had code to set the TTL in handleResult for the _isAborted case, which is probably why it doesn't cancel the stream - it's easier just to let it finish. It might be reasonable, in another bug that's going to have lots of testcases, to get the caching info off the stream, set the TTL, and cancel the stream, so we don't load all of a huge feed that's malformed in the first few characters, but that would be another bug.

Comment 24

[Phil Ringnalda (:philor)]

Ah, and we don't want to cancel the stream and thus doom the cache entry, since for all we know it's almost done and will give us an etag that will let us avoid refetching the same busted thing over and over again for days.

Comment 25

[(no longer active)]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/0332f1e3024f

Comment 26

[Tanner M. Young [:tmyoung]]

Verified Fixed.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a2pre) Gecko/20090821 Namoroka/3.6a2pre (.NET CLR 3.5.30729) ID:20090821052443