A technically correct install.rdf will be rejected because it does not match exactly what the regular expression is looking for. By using XML parsing with a schema, we will be better at rejecting invalid install.rdfs (where things are nested improperly) and valid install.rdfs (that place the namespace differently).

Bulk Moving Developer Control Panel bugs to new component. (Filter: massdevcpspam)

This caused bug 273550 A simple linebreak causes the GUID to be picked up wrong.

Patches welcome, until somebody takes this bug, I expect the bug settings not to be changed.

Is there any point in making a patch now? Have you landed all of the mass changes you're going to make? I don't want to do work twice.

This sucks. My Extension Developer extension uses Mozilla's RDF serializer for its install.rdf editor, which produces install.rdf files that UMO can't handle. Editing install.rdf by hand sucks.

This looks promising http://phpxmlclasses.sourceforge.net/show_doc.php?class=class_rdf_parser.html

Taking, patch coming right up.

This uses the RDF class alanjstr linked above (which is a single php file). I stuck the RDF class in core/, and this file gets included from additem.php.

The rest.

Pike, can you sanity check my RDF parsing?

Comment on attachment 168377 [details] [diff] [review] Changes to additem, parse install.rdf with an RDF parser instead of regexes. >Index: additem.php <...> >@@ -297,21 +256,21 @@ <...> >- >+a Really?

Ted - As Pike pointed out, it looks like you fat-fingered something. Pike - How's the rest of it look?

Comment on attachment 168376 [details] parse_install_manifest.php I have some issues with this, mostly l18y. Anyway, those are way-future. See bug 274068 for the pending support of xml:lang within RDF. On the code itself, I don't know RDF in php. But this seems to be a good improvement. Hoping that the rdf lib is ok. Oh, there may be issues where the Mozilla RDF parser accepts stuff that a standards compliant parser wouldn't. But this shouldn't matter for RDF that adheres to the extension manager grammar, as those validate by now.

Please use escape_string() when you grab each item.

This doesn't look bad to me, at a glance anyway. Has this been tested to ensure it doesn't break on existing items? (Including those that the site currently breaks on, but more importantly, those it doesn't) If it's been thouroughly tested (or can be in the next day or so), then I'm ok with getting it reviewed and in for 1.0, otherwise it'll wait till 1.1.

I've tested it on a variety of install.rdf files, both from my install.rdf editor, and in the "standard" format accepted by UMO. If you have any edge cases you'd like me to test, I can certainly do that. I'll get a cleaned up patch attached tomorrow.

Nah, I'm mostly concerned with the mainstream cases, those generated by your extension (which are correct) and the "standard" that we already handle gracefully. :-)

Fixes the typo, fixes the one case where manifest data was used without escape_string(). Axel, if you have i18n/l10n/whatever issues with this, please file other bugs on me. Thanks!

Moving big changes to 2.0.

Comment on attachment 169753 [details] [diff] [review] Updated patch Requesting R from CTho. Don't forget to grab the php class, too.

Comment on attachment 168376 [details] parse_install_manifest.php What if someone wants to include descriptions in more than one language? Does this require http://www.mozilla.org/2004/em-rdf# to be available? If so, what if the site is down? Why did you define $l = strlen(EM_NS); in the first function?

Comment on attachment 169753 [details] [diff] [review] Updated patch I'm assuming parse_install_manifest() does what it claims when I do this review. - if ($release == $val[minversion]) { $versioncheck[$key][minversion_valid] = "true"; } - if ($release == $val[maxversion]) { $versioncheck[$key][maxversion_valid] = "true"; } + if ($release == $val["minVersion"]) { $versioncheck[$key][minversion_valid] = "true"; } + if ($release == $val["maxVersion"]) { $versioncheck[$key][maxversion_valid] = "true"; } Why didn't you add quotes around minversion_valid / maxversion_valid? Any reason you didn't capitalize min/maxversion_valid to be consistent with minVersion and maxVersion? if ($versioncheck[errors]=="true") { Should errors be in quotes? - $minappver = $manifestdata["targetapplication"]["$guid"]['minversion']; - $maxappver = $manifestdata["targetapplication"]["$guid"]['maxversion']; + $minappver = $manifestdata["targetApplication"]["$guid"]['minVersion']; + $maxappver = $manifestdata["targetApplication"]["$guid"]['maxVersion']; Could you change the single quotes to double quotes as well? +foreach ($manifestdata["targetApplication"] as $key=>$val) { //echo"$key -- $val[minversion] $val[maxversion]<br>\n"; Remove the comment, or change the capitalization so it will work when someone uncomments it.

Changed to support xml:lang for name and descriptions. Also returns null on parse errors now.

Fixed per ctho's comments. Also changed to fit with the xml:lang changes in parse_install_manifest().

Comment on attachment 171557 [details] [diff] [review] parse_install_manifest.php Why aren't we ignoring iconURL, optionsURL, and aboutURL? I'm glad you snagged updateURL because we'll need it for something else. Why is there a commented code line //$rdf->rdf_set_warning_handler("mf_warning_handler" ); Why do you define $l = strlen(EM_NS); inside of parse_install_manifest() ?

(In reply to comment #25) > (From update of attachment 171557 [details] [diff] [review] [edit]) > Why aren't we ignoring iconURL, optionsURL, and aboutURL? Good question. > > I'm glad you snagged updateURL because we'll need it for something else. > > Why is there a commented code line > //$rdf->rdf_set_warning_handler("mf_warning_handler" ); Because I was using a warning handler but I removed it. I can remove the comment. > Why do you define $l = strlen(EM_NS); inside of parse_install_manifest() ? Because I use the value twice, it saved me some typing and an extra calculation.

Once more, with feeling.

For real this time.

This is the RDF parsing class. It should be added to core/

Checking in mozilla/webtools/update/core/class_rdf_parser.php; /cvsroot/mozilla/webtools/update/core/class_rdf_parser.php,v <-- class_rdf_parser.php initial revision: 1.1 done RCS file: /cvsroot/mozilla/webtools/update/developers/parse_install_manifest.php ,v done Checking in mozilla/webtools/update/developers/parse_install_manifest.php; /cvsroot/mozilla/webtools/update/developers/parse_install_manifest.php,v <-- parse_install_manifest.php initial revision: 1.1 done Checking in mozilla/webtools/update/developers/additem.php; /cvsroot/mozilla/webtools/update/developers/additem.php,v <-- additem.php new revision: 1.12; previous revision: 1.11 done

Still needed on Branch.

Branch: Checking in mozilla/webtools/update/developers/additem.php; /cvsroot/mozilla/webtools/update/developers/additem.php,v <-- additem.php new revision: 1.11.2.1; previous revision: 1.11 done Checking in mozilla/webtools/update/developers/parse_install_manifest.php; /cvsroot/mozilla/webtools/update/developers/parse_install_manifest.php,v <-- p arse_install_manifest.php new revision: 1.1.2.1; previous revision: 1.1 done Checking in mozilla/webtools/update/core/class_rdf_parser.php; /cvsroot/mozilla/webtools/update/core/class_rdf_parser.php,v <-- class_rdf_par ser.php new revision: 1.1.2.1; previous revision: 1.1 done