Thanks Heather, and thanks Gary for the implementation of CmdUtils.parserVersion in 0.5. :)
There's a few reasons we decided to go the CmdUtils.parserVersion route. The other solution we considered was requiring the <LINK REL> tag on the HTML page to specify the version of the parser a feed was written for, e.g.: <LINK REL="commands" VERSION="1" HREF="foo-parser1.js"/> <LINK REL="commands" VERSION="2" HREF="foo-parser2.js"/> These are the reasons we didn't go the above route: * Because Ubiquity doesn't currently support modules or any other kind of code-sharing system, requiring command authors to provide completely separate feeds for different parser versions would probably require them to copy-and-paste common code between the two feeds, which violates DRY [1]. * There's currently a bug in Ubiquity whereby after subscribing to a command feed, Ubiquity never checks the HTML page that contains the <LINK REL> tag again; it only checks the .js file pointed to by the <LINK REL> tag for updates. This bug, in addition to Ubiquity's general architecture surrounding command feeds, means that actually implementing the VERSION attribute would require a lot of implementation effort to work smoothly. * By default, github and other services that host command feeds would have to change their behavior if we went the VERSION route. * Python's standard library uses top-level "if" statements that examine static conditions that won't change throughout the process lifetime (e.g., the host OS) to define functions differently depending on the conditions. Actually changing the parser version in ubiquity requires a restart, so examining CmdUtils.parserVersion satisfies the same kind of criteria. In other words, although it's not super- elegant, it's something that's done in lots of production code out there and there's nothing really "wrong" with it. * The "VERSION" attribute isn't valid HTML. Firefox would be able to read it and work with it, but it'd interfere with folks who run their code through HTML validators and such. We also considered using the "REV" attribute, which we thought would be short for "revision", but that's actually not what it's intended to be used for in the HTML specificaiton. * To respond to Timofei's concern, the Herd will be able to figure out which feeds are parser1 vs. parser2 using the CmdUtils.parserVersion method. This is because, in order to figure out what commands a feed has, the Herd actually executes the feed in a restricted sandbox environment. Figuring out what commands a hybrid feed provides to parser 1 and parser 2 will just involve evaluating the code twice: once with CmdUtils.parserVersion set to 1, and again with CmdUtils.parserVersion set to 2. I hope this clarifies things for everyone, let us know if you have any other questions though! :) - Atul [1] http://en.wikipedia.org/wiki/Don%27t_repeat_yourself On Jul 10, 6:14 am, Heather <[email protected]> wrote: > Actually, CmdUtils.parserVersion was added to 0.1.9 too, and will > always return 1. You are right that it will break for versions prior > to 0.1.9. > > On Jul 10, 3:55 am, garyhodgson <[email protected]> wrote: > > > Hi, > > > I'm glad that the parserVersion hack is useful, but it's worth noting > > that this is only useful for people who have upgraded to 0.5 or > > greater. To cater for people who have not upgraded, the function > > still has to be defined in the command itself. And if you do > > implement it in the command itself please note the following... > > > The function added in CmdUtils (https://ubiquity.mozilla.com/hg/ > > ubiquity-firefox/rev/651ba8eea7bf) leaves out the following check: > > > return (branch.getPrefType("parserVersion") != 0) ? > > branch.getIntPref > > ("parserVersion") == 2 : false; > > > Versions prior to 0.5 of course do not have this parserVersion > > preference and so getIntPref fails with an exception: > > > [Exception... "Component returned failure code: 0x8000ffff > > (NS_ERROR_UNEXPECTED) [nsIPrefBranch.getIntPref]" nsresult: > > "0x8000ffff (NS_ERROR_UNEXPECTED)" location: "JS frame :: file:/// > > ... /extensions/[email protected]/modules/ > > sandboxfactory.js#ubiquity://command-editor-code :: parserVersion :: > > line 5" data: no] > > > nsIPrefBranch seems to lack some basic functionality such as a > > "hasPref" function, or even a nice degradation on the getPref > > functions, and so using the following function works with versions > > prior to 0.5: > > > function parserVersion() { > > var prefs = > > Components.classes["@mozilla.org/preferences-service; > > 1"].getService(Components.interfaces.nsIPrefService); > > var branch = prefs.getBranch("extensions.ubiquity."); > > return (branch.getPrefType("parserVersion") != 0) ? > > branch.getIntPref > > ("parserVersion") == 2 : false; > > } > > > Cheers, > > Gary --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "ubiquity-firefox" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, visit this group at http://groups.google.com/group/ubiquity-firefox?hl=en -~----------~----~----~----~------~----~------~--~---
