Closed Bug 648978 Opened 14 years ago Closed 13 years ago

install.rdf parsing in Talos is very rudimentary and fails for some add-ons

Categories

(Testing :: Talos, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: ecfbugzilla, Assigned: anodelman)

References

Details

Attachments

(1 file)

Talos fails to extract extension ID from the install.rdf file in Tab Mix Plus, all tests for this extension "crash" because of that. The file is valid but uses slightly untypical RDF: <?xml version="1.0"?> <RDF:RDF xmlns:NS1="http://www.mozilla.org/2004/em-rdf#" xmlns:NC="http://home.netscape.com/NC-rdf#" xmlns:RDF="http://www.w3.org/1999/02/22-rdf-syntax-ns#"> <RDF:Description RDF:about="rdf:#$n83In3" NS1:id="{ec8030f7-c20a-464f-9b0e-13a3a9e97384}" NS1:minVersion="3.5" NS1:maxVersion="4.0.*" /> <RDF:Description RDF:about="urn:mozilla:install-manifest" NS1:id="{dc572301-7619-498c-a57d-39143191b318}" NS1:name="Tab Mix Plus" NS1:version="0.3.8.5" NS1:type="2" NS1:unpack="true" NS1:description="Tab browsing with an added boost." NS1:iconURL="chrome://tabmixplus/skin/tmp.png" NS1:homepageURL="http://tmp.garyr.net" NS1:optionsURL="chrome://tabmixplus/content/pref/pref-tabmix.xul" NS1:aboutURL="chrome://tabmixplus/content/about.xul" NS1:creator="ONEMEN (tabmix.onemen@gmail.com)" NS1:developer="ONEMEN"> <NS1:targetApplication RDF:resource="rdf:#$n83In3"/> </RDF:Description> </RDF:RDF>
Thanks for the example install.rdf.
Assignee: nobody → anodelman
This is covering the same ground as bug 648732, so I have this as the patch to bug 648732 plus some further dom parsing. Tested on tools-r3-leopard-001 and it was able to install tabmixplus while not losing the ability to install other addons.
Attachment #529802 - Flags: review?(jmaher)
Comment on attachment 529802 [details] [diff] [review] [checked in]add xml parsing for tabmixplus Is that wise? RDF is a complex format, trying to parse it with an XML parser means looking for trouble. I understand that installing a new Python library on all Talos machines probably isn't feasible but rdflib can simply be added to the Talos repository as rdflib subdirectory. Then you could do something like: > import rdflib > > ... > > manifest = rdflib.Graph() > f = open(os.path.join(addon, 'install.rdf')) > manifest.parse(f) > f.close() > > em = rdflib.Namespace('http://www.mozilla.org/2004/em-rdf#') > id = None > unpack = 'false' > roots = [rdflib.URIRef('urn:mozilla:install-manifest')] > # rdflib doesn't recognize node id correctly if RDF is the default namespace > roots.extend(manifest.subjects(rdflib.URIRef('about'), 'urn:mozilla:install-manifest')) > for root in roots: > for idLiteral in manifest.objects(root, em['id']): > id = idLiteral > for unpackLiteral in manifest.objects(root, em['unpack']): > unpack = unpackLiteral
I would guess that we don't know the amount of addons that are using rdf formatting? Yes, installing another lib is an annoyance but would be worth while if it handles more cases... I was willing to special case TabMixPlus as I had assumed that it was an isolated case.
A quick search on the add-ons MXR (I looked for ":id=" in install.rdf files) yields at least a couple hundred results. It's rare, yes, but I think it's best to implement a more reliable solution considering that we want to extend these checks to more add-ons.
Comment on attachment 529802 [details] [diff] [review] [checked in]add xml parsing for tabmixplus Review of attachment 529802 [details] [diff] [review]: overall this looks good, just want to avoid python traceback if we try to access an object that doesn't exist. I assume the elem.hasAttribute(..) will enforce the check? ::: ffsetup.py @@ +145,5 @@ addon_id = str(elem.getAttribute('em:id')) + else: + if ((elem.hasAttribute('RDF:about')) and (elem.getAttribute('RDF:about') == 'urn:mozilla:install-manifest')): + if elem.getElementsByTagName('NS1:id'): + addon_id = str(elem.getElementsByTagName('NS1:id')[0].firstChild.data) any chance we could have a bogus file and firstChild is null?
Attachment #529802 - Flags: review?(jmaher) → review+
The 'getElementsByTagName' ensures that there will be something in firstChild. I think that we can go with this solution for now as a stop gap, and do full rdf support in a separate bug.
Comment on attachment 529802 [details] [diff] [review] [checked in]add xml parsing for tabmixplus changeset: 235:d1afa96e783d
Attachment #529802 - Attachment description: add xml parsing for tabmixplus → [checked in]add xml parsing for tabmixplus
Depends on: 651670
Further work to correctly parse rdf will be done in bug 655171.
Deployed.
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: