389943 - loading microsummary generators throws an exception (breaks microsummaries)

Status: VERIFIED FIXED

(Firefox Graveyard :: Microsummaries, defect)

Description

[Steve England [:stevee]]

Attachment: miranda.xml file for microsummary

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a7pre) Gecko/200707270505 Minefield/3.0a7pre

1. Start firefox, make a new profile, quit firefox
2. In your profile dir, make a directory called 'microsummary-generators'
3. Place miranda.xml in the directory (from attachment)
4. Start firefox up

Error console reports:

Error: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIXMLHttpRequest.setRequestHeader]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: file:///C:/Documents%20and%20Settings/Ste/Desktop/Firefox%20Trunk%20(PLACES)/firefox/components/nsMicrosummaryService.js :: MSR_load :: line 1991"  data: no]
Source File: file:///C:/Documents%20and%20Settings/Ste/Desktop/Firefox%20Trunk%20(PLACES)/firefox/components/nsMicrosummaryService.js
Line: 1991

Error: uncaught exception: [Exception... "Component returned failure code: 0x80570016 (NS_ERROR_XPC_GS_RETURNED_FAILURE) [nsIJSCID.getService]"  nsresult: "0x80570016 (NS_ERROR_XPC_GS_RETURNED_FAILURE)"  location: "JS frame :: chrome://browser/content/browser.js :: delayedStartup :: line 943"  data: no]

5. Navigate to http://www.miranda-im.org/development/
6. Bookmarks > Bookmark this page

Error console reports:

Error: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIXMLHttpRequest.setRequestHeader]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: file:///C:/Documents%20and%20Settings/Ste/Desktop/Firefox%20Trunk%20(PLACES)/firefox/components/nsMicrosummaryService.js :: MSR_load :: line 1991"  data: no]
Source File: file:///C:/Documents%20and%20Settings/Ste/Desktop/Firefox%20Trunk%20(PLACES)/firefox/components/nsMicrosummaryService.js
Line: 1991

Expected:
The Name field should have a drop-down menu where I can change the bookmark name to the microsummary

Actual:
No drop-down menu, so no way to change the bookmark name to the microsummary.

(Also, if you choose to add the bookmark after step 6, then right click on the bookmark you just made, three more warnings are shown in the error console. Once you dismiss the right-click context menu, another error is added)

Comment 1

[Steve England [:stevee]]

Works 20070726_1513_firefox-3.0a7pre.en-US.win32.zip
Broken 20070726_1530_firefox-3.0a7pre.en-US.win32.zip

http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&date=explicit&mindate=1185487980&maxdate=1185488999

Due to bug 389508

Comment 2

[Steve England [:stevee]]

This bug might be causing bug 389948.

Comment 3

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

How does adding the file manually differ from adding it through the Firefox UI? Presumably the files end up in the same place on disk; do their contents differ?

Comment 4

[Steve England [:stevee]]

When I add a microsummary through the firefox UI, nothing is written to the ../<firefox profile>/microsummary-generators/ directory. I presume whatever makes a microsummary tick gets written straight into places.sqlite if it is supplied by the webpage itself.

When adding a microsummary manually, you have to put the .xml file in the ../<firefox profile/microsummary-generators/ directory, and then bookmark the page and tell firefox to use the microsummary title instead of the static webpage title; in this case I guess it's importing the microsummary .xml file or something, and this is where it's failing.

But really, I don't know the exact ins and outs of this process.

Comment 5

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

I'm not really familiar with the Microsummary code either, but it looks like the "add new generator" code does try to add a file on disk, next to manually installed generators (http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/browser/components/microsummaries/src/nsMicrosummaryService.js&rev=1.68&mark=468#451). 

Could it be that the patch for bug 389508 just broke setRequestHeader unconditionally from JS components (perhaps because of incorrect privilege checks?), and that the microsummary code that does that is only called for "manually installed" generators?

Comment 6

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

The code checks for "UniversalBrowserRead" when .open is called. Does the microsummary code call .openRequest directly by any chance? And does having UniversalXPConnect also give you UniversalBrowserRead?

Comment 7

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

We're hitting the case at http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/content/base/src/nsXMLHttpRequest.cpp&rev=1.191&mark=2071#2051 , added in the patch for bug 389508, because the Microsummary service is trying to use an XMLHTTPRequest to retrieve a file:// URI.

I'm not sure whether this is "supported", or what should happen in this case.

Comment 8

[Bill Gianopoulos [:WG9s]]

Despite previous comments to the contrary, this works no better if the microsummary generator is installed using the UI.  I had the Yahoo Finance generator whichs shows the current price plus daily change on stock quote pages form this page http://people.mozilla.com/~myk/microsummaries/generators/installed, and was seeing this issue.

Just to make sure, I renamed the microsummary-generators folder in me profile and removed the offending bookmark and made certain that restored normal behavior.

Then, using a trunk build from before bug 389508 landed, I went to http://people.mozilla.com/~myk/microsummaries/generators/ and clicked on the install link for that generator.

I then visited the desired stock quote page and bookmarked it selecting the microsummary generated title and the bookmark toolbar as the location for the bookmark.  I then exited and relaunched this version of Firefox and verified everything still worked as expected.

I then tried the current trunk build using the same profile and got the same results as before.  The same errors described above in the error console, non functioning microsummaries, no session restore and unable to undo a close tab.

If there is some outer UI in Firefox that I should be using to install this, it is certainly not documented in the WIKI or the help file nor is it easily discoverable. 

Comment 9

[Bill Gianopoulos [:WG9s]]

(In reply to comment #7)
> We're hitting the case at
> http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/content/base/src/nsXMLHttpRequest.cpp&rev=1.191&mark=2071#2051
> , added in the patch for bug 389508,


The old code was:

if (!mChannel)             // open() initializes mChannel, and open()
  return NS_ERROR_FAILURE; // must be called before first setRequestHeader()

so the comment makes sense.

The new code is:

if (!httpChannel)          // open() initializes mChannel, and open()
  return NS_ERROR_FAILURE; // must be called before first setRequestHeader()

Either the if should be reverted to be testing mChannel, or the comment should be changed to say that httpChannel is initialized upon open.  The comment makes no sense based on the current code.

Comment 10

[Steve England [:stevee]]

So maybe there is a differentiation to be made between 'microsummary generators' (third party sites that generate a microsummary for another site) and the kind of microsummary that is embedded in a webpage.

Comment 11

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

Ah, yeah. Any installed "generator" should trigger this problem, whether it was added manually or not.

Comment 12

[Bill Gianopoulos [:WG9s]]

(In reply to comment #9)
> (In reply to comment #7)
> > We're hitting the case at
> > http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/content/base/src/nsXMLHttpRequest.cpp&rev=1.191&mark=2071#2051
> > , added in the patch for bug 389508,
> 
> 
> The old code was:
> 
> if (!mChannel)             // open() initializes mChannel, and open()
>   return NS_ERROR_FAILURE; // must be called before first setRequestHeader()
> 
> so the comment makes sense.
> 
> The new code is:
> 
> if (!httpChannel)          // open() initializes mChannel, and open()
>   return NS_ERROR_FAILURE; // must be called before first setRequestHeader()
> 
> Either the if should be reverted to be testing mChannel, or the comment should
> be changed to say that httpChannel is initialized upon open.  The comment makes
> no sense based on the current code.
> 

Ignore this comment!

I must be going blind if I had just looked one line earlier at the new code I would have seen:

  nsCOMPtr<nsIHttpChannel> httpChannel(do_QueryInterface(mChannel));
  if (!httpChannel)          // open() initializes mChannel, and open()
    return NS_ERROR_FAILURE; // must be called before first setRequestHeader()


So the comment still makes sense.

Comment 13

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

The old code used to simply be a no-op if you called setRequestHeader after open()ing a file uri. The new code throws an exception. I'll revert to making it a no-op.

Comment 14

[Bill Gianopoulos [:WG9s]]

Attachment: alternative solution

This could also be fixed this way in the microsummary code.  But then, I wonder if there are other places that might depend on the old behavior.

Comment 15

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Fixed by patch in bug 389508

Comment 16

[Steve England [:stevee]]

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a7pre) Gecko/2007073105 Minefield/3.0a7pre ID:2007073105
VERIFIED