Cannot create new ticket in bug tracker/modifying Function prototype has...
Index the web pages you visit with the Recoll text search tool
Status: Abandoned
Brought to you by:
dlk003
I wanted to file this as a ticket, but I can't add new tickets for some reason. utils.js modifies Function.prototype to add a new function, but does so as an enumerable property, which means it shows up when iterating through objects. I think it's better to add the prototype functions as non-enumerable properties: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/defineProperty
Specifically, if you change https://sourceforge.net/p/recollfirefox/code/ci/default/tree/chrome/content/utils.js#l58 to
Object.defineProperty(Function.prototype, 'recoll_bind', {
value: function(f,obj) {
var temp = function() {
return f.apply(obj, arguments);
};
return temp;
},
enumerable: false
});
the problem would be solved. Functions such as bind or recoll_bind should not be enumerable properties of Function.
Adding things to Function prevents automatic signing for Mozilla extensions. I had a monkey-patch in my own extension that moved recoll_bind to an innumerable property as shown above, but that means I must add things to Function, which means my own extension cannot be signed automatically. Zotero bombs out with the existing recoll_bind, which is why I was trying to work around it.
Hi,
The maintainer for recoll-firefox does not seem to be around. I (the Recoll developer) seem to have the admin rights on the project, so I should be able to commit code and produce a new release, but I know nothing about javascript.
Friflaj, I am not sure that I understand what you are saying. Do you think that there is a possible solution to this issue or not ?
jf
I'm still here JF. I don't think that friflaj is reporting a problem. I read this as a suggestion that will help get the recoll Firefox extension through the Mozilla extension signing process. I have not had time to spend on that bureaucratic hurdle yet.
Last edit: Dave King 2015-07-25
Hi Dave,
I had ended up thinking that you were not around any more, so I took the plunge into javascript, and made quite a few modifications to the code to make it conform to the Mozilla rules for inclusions in the addons site and signing (as you can see in the changelog).
The version matching the latest commits has been submitted to the Mozilla addons and is awaiting reviewing (it passes the automatic checks with no errors);
https://addons.mozilla.org/en-US/firefox/addon/recoll-indexer-1/
I've removed a significant amount of code for functionality which did not seem to work (e.g, I've removed "index this link", which did not work for me, and all search functions which quite certainly worked for nobody).
Maybe I've removed too much, so it would be nice if you could try the extension, and tell me if something needs to be reinstated.
We can update the package while it sits in the queue, and it will be a few weeks before it is actually reviewed, so we have time to fix what needs to be.
I've also modified the tickets section permissions: several users have complained about not being able to create tickets, and I don't think that we are in danger of being submerged in tickets...
Cheers,
jf
Last edit: Jean-Francois Dockes 2015-07-25
Also: I saw in the precedent topic that you created a taskbar button for replacing the status-bar one (rip). I did not see a trace of this in the code. Maybe you did not push your changes. I guess that we should be able to merge this in the current version.
Lastly: you need to uninstall the old extension before installing the new one. I changed the extension id because the Mozilla site complained about duplicates (because of your earlier submission I guess).
So I should delete my Add-On from the Mozilla AOM then? You have one of your own up there now?
I'll put the toolbar button code back in.
You did not put the 2.0.1 XPI file up for download here in SourceForge?
If your version currently on AOM has the code which modifies the standard object prototypes, I think that you can delete it, as far as I understand, it won't go through. Where is it in the review queue ? If it's closed to being reviewed, maybe we could just update it with the new version (in which we'd restore the extension id)
About the button code, it's a pity you had not pushed it to sourceforge, I'm afraid that it will have to be merged back in manually, there have also been a lot of indentation changes. If you send me your current version, I volunteer for doing the merge, as I'm the guilty one here.
I have not put the 2.0.1 XPI file on sourceforge, because I'd like it to get a bit more testing before publication. It's currently hidden from public searches on the Mozilla site, but I have pointed Jiang to it, and I could ask for testers on the Recoll mailing list. We can put it on sourceforge as soon as we are confident (and the button is back).
I wanted it in the queue asap because it will take around 10 weeks before it's approved and published, plenty of time to test and update.
Last edit: Jean-Francois Dockes 2015-07-25
The source code with the toolbar included is here in the 1.3.0 XPI on SourceForge, if you just unzip that. I looked at the code you commited and from what I can see you'll need to put back all of the status bar icon code that you removed, since that is what the toolbar button uses also, and update the overlay XUL with a stanza for the toolbar button. I'd suggest putting the status bar icon back in there too, Some people (like me) prefer the old style status bar icons to the new style toolbar buttons. I've installed a status bar extension just so that I can use the old style status bar icons in several extensions.
Other things I thought of: There's an collection of multiple sized icons that goes with a toolbar button for the browser configuration pages. You'll see that in my XPI. You'll need to take a look at the private browsing code that you took out too, as we have to account for someone switching in and out of private mode while toggling recoll indexing state using the button or icon.
Ok, I think that I've merged all the button code. The source is in the repository, and I have uploaded 2.1.1 to https://addons.mozilla.org/en-US/firefox/addon/recoll-indexer-1/
Also I've made a change to the page load event function, which seems to improve indexing on some sites (following a report that nytimes.com was not indexed).
Seems like there are some bugs. I loaded 2.1.2 and the buttons showed indexing as being active, but it was not. The recoll.autoindex.active preference was set to false and it showed as unchecked in the preferences pane. According to the button tooltips however, auto indexing was active. Clicking the buttons would make the button tooltips and icons change state, but not the underlying preference setting. After fiddling with the preferences pane for a minute, all of a sudden everything, including the buttons, started working. Very odd. Then, after getting it turned on, I went to a new page in the browser and that produced a popup error indicating a Java script error during indexing. This error message was written out to the console when I subsequently tested this in my development browser:
[recoll] Page Loaded
[recoll] Should Index ? Exclude list match: false. Include list match: false
[recoll] indexPage: queuing for indexing: http://start.fedoraproject.org/
[recoll] indexPage: WriteContent/Metad. failed: [Exception... "It's illegal to pass a CPOW to native code arg 0 [nsIWebBrowserPersist.saveDocument]" nsresult: "0x80570036 (NS_ERROR_XPC_CANT_PASS_CPOW_TO_NATIVE)" location: "JS frame :: chrome://recoll/content/recollOverlay.js :: recoll.writeContent :: line 370" data: no]
About the savedocument error: What firefox version are you using ?
I'll try to check the logic between prefs and buttons tomorrow.
Further tests showed me that the error message is not present in Firefox 39, but is in Firefox 41.a02. Both 39 and 41 have the odd button behaviour the first time that the extension is used.
I wonder if the Firefox 41 error might be related to the e10s changes. Is electrolysis enabled in your browser ?
I think that I've fixed the icon initialisation issue. 2.1.3 is on the addons site.
yes, electrolysis is on in 41 and yes, turning it off does make the problem go away.
the icon seems to be working fine now.
I am not surprised that things don't work with e10s. The error happens in code which did not change recently, by the way (writeContent()).
I'm not too sure how we are supposed to do this if this method does not work with e10s, I guess we'll have to ask the question (a quick google search did not find an answer). Do you want to pursue this or should I do it ?
Did you receive my email about indentation by the way ?
Sorry not to have responded more quickly, real life interveened. I'm not sure how to deal with the e10s changes either. I'm fine if you want to take the lead on this extension. I have very little time to devote to it at present. (I did see your note about indentation and I had no concerns about that.)
Hi,
The extension has been reviewed and is now available from the Mozilla add-ons site:
https://addons.mozilla.org/en-US/firefox/addon/recoll-indexer-1/
It should hopefully be usable with the near-future versions which will enforce signature verification.
It is still not compatible with multi-process Firefox (electrolysis, e10s), but I hope that passing the review hurdle gives us a little time, which will be much needed, because quite a few things are going to need changing, with Mozilla apparent plans for deprecating all the stuff we are depending on. Hopefully they won't be too hasty for this (else they are going to make quite a few people angry, there are quite a few extensions much more important than recoll-firefox which depend on this stuff)...
I am wondering if the old xpi files on sourceforge should be archived away? They are a source of confusion.
jf
On 08/27/2015 12:54 PM, Jean-Francois Dockes wrote:
Yes, they should go away. Do you want to do the necessary, or do you
need me to do that?
--
David King
dave at daveking dot com
I've done it. I've put aside copies of the old packages in case they are ever needed.
jf