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)
Testing
Talos
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: ecfbugzilla, Assigned: anodelman)
References
Details
Attachments
(1 file)
(deleted),
patch
|
jmaher
:
review+
|
Details | Diff | Splinter Review |
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>
Assignee | ||
Comment 2•13 years ago
|
||
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)
Reporter | ||
Comment 3•13 years ago
|
||
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
Assignee | ||
Comment 4•13 years ago
|
||
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.
Comment 5•13 years ago
|
||
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 6•13 years ago
|
||
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+
Assignee | ||
Comment 7•13 years ago
|
||
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.
Assignee | ||
Comment 8•13 years ago
|
||
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
Assignee | ||
Comment 9•13 years ago
|
||
Further work to correctly parse rdf will be done in bug 655171.
Assignee | ||
Comment 10•13 years ago
|
||
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.
Description
•