Currently, there is no easy way to extract an entity from a dtd file. This means that we cannot check localized strings. This bug is a tracking bug for implementing a getEntity() method in UtilsAPI.

This blocks bug 504497 as I need to compare the tab title against the certerror.pagetitle entity in aboutCertError.dtd.

Any idea how to do that? I know that this could be hard to do with JS but otherwise how we should check for correctness?

One way to do it would be to create a data url for something like <?xml version="1.0"?> <!DOCTYPE elem [ <!ENTITY % brandDTD SYSTEM "chrome://branding/locale/brand.dtd" > %brandDTD; ]> <elem>&brandShortName;</elem> but that requires you to know all DTD chrome urls. I don't see a way to get those by DOM inspection of the generated document.

That's the way what Benjamin has proposed too. There is no need to know about all the DTD's inside this helper function. It will simply be a parameter which gets folded in like: getEntity(chromeURL, entityName).

Oh and we would have to open that data url in a hidden window to not cause a focus switch to another window.

You'll need a list of chromeURLs, as some DTDs reference others, in particular, brand.dtd. There are more, though. And you won't need a window at all, using just DOMParser.parseFromString should do it.

Code snippet: p = new DOMParser(); d=p.parseFromString('<?xml version="1.0"?><!DOCTYPE elem [<!ENTITY % brandDTD SYSTEM "chrome://branding/locale/brand.dtd" >%brandDTD;]><elem id="brandShortName">&brandShortName;</elem>','text/xml') [object XMLDocument] $('elem[id=brandShortName]',d).text() Minefield With the id stuff, you can actually get out multiple values in one document, if that's required.

A patch based on Pike's approach from Comment #7

Comment on attachment 439521 [details] [diff] [review] patch based on Pike's approach >+ * Called to get the value of an individual entity in a DTD file. >+ * >+ * @param {array} url >+ * An array of string urls. You should use [string] as the type so we also can see the parameter type. I would also prefer to have a better description which at least include that we have URLs to DTDs here. >+function getEntity(urls, entityId) >+{ >+ var entities = ""; >+ for (i = 0; i < urls.length; i++) { >+ entities += '<!ENTITY % dtd' + i + ' SYSTEM "' + urls[i] + '">%dtd' + i + ';'; >+ } Here we are building a string of external entities. Can you please reflect that in the variable name? >+ var doc = parser.parseFromString(header + elem, 'text/xml'); >+ return doc.querySelector('elem[id="elementID"]').textContent; >+} Can you please add an empty line before the return statement and split this one into two lines? That would be better readable. Also make sure that the new function matches the alphabetical order of all functions in that file. With those changes, r=me.

taking r+ from previous patch

Comment on attachment 441399 [details] [diff] [review] patch v2 Adrian, what about the additional dtd, which should be included by default as what Axel has been proposed? That information arrived after I have reviewed your last patch.

(In reply to comment #11) > Adrian, what about the additional dtd, which should be included by default as > what Axel has been proposed? That information arrived after I have reviewed > your last patch. Right, I forgot about it. The question here is, if it should be included by default in the array, or if a comment about the possible necessity of including the xhtml11.dtd would be enough?

Can you please tell again why this dtd is needed? Given our short discussion via email it is not clear for me which purpose it has.

When loading an xhtml document, the parser silently add xhtml11.dtd to include the html pre-defined entities, like &auml;. Some localizers use those in their dtd entities, and sometimes the code does. Like &reg;, for example.

Ok, given that we should always include this dtd. We shouldn't force the author to add it to the test itself. Axel, the only downside for the general inclusion would be some milliseconds for parsing this DTD, right?

xhtml11.dtd is only included for xhtml docs, not for xul docs. So "always" isn't strictly true. But I doubt it'll be bad for the tests if it's always on.

based on previous comments, it looks like always adding the xhtml11.dtd is the best idea. The only change in this patch in compare to the previous one is to add the xhtml11.dtd by default

Comment on attachment 444210 [details] [diff] [review] patch v3: add xhtml11.dtd by default > /** >+ * Called to get the value of an individual entity in a DTD file. nit: Please use "Returns the value...". You can also update the heading of getProperty. >+ var extEntities = ""; >+ // Build a string of external entities >+ for (i = 0; i < urls.length; i++) { nit: Move the comment before the variable declaration. >+ extEntities += '<!ENTITY % dtd' + i + ' SYSTEM "' + urls[i] + '">%dtd' + i + ';'; nit: Try to limit it to 80 characters. With those updates you will get the final r=me.

addressing review comments from Comment 18

Comment on attachment 444270 [details] [diff] [review] patch v4 + extEntities += '<!ENTITY % dtd' + i + ' SYSTEM "' + >+ urls[i] + '">%dtd' + i + ';'; >+ } >+ var parser = Cc["@mozilla.org/xmlextras/domparser;1"] The alignment of the wrapped line is not correct and an empty line is missing after the for loop. But I will fix that with the check-in.

Landed on all branches as: http://hg.mozilla.org/qa/mozmill-tests/rev/38e5b63d2d4d http://hg.mozilla.org/qa/mozmill-tests/rev/0baec063da46 http://hg.mozilla.org/qa/mozmill-tests/rev/bd25448c9312 Adrian, can you also please update the documentation for that new function?

(In reply to comment #21) > http://hg.mozilla.org/qa/mozmill-tests/rev/38e5b63d2d4d Benjamin, as you can hopefully remember we talked about fetching entities from DTD files during our last all hands in December. Now we have checked-in a solution for Mozmill only. Could you have a look at the checked-in source and tell me if we wanna have that function in our core code? If yes, what's the best place for it? Should it only be a part of the Mochitest framework or do we wanna see it related to string bundles?

That's a question for Axel really, but I don't know of a reason why we'd want that in our code now. l20n basically solves the entire issue, as I understand it.

(In reply to comment #23) > That's a question for Axel really, but I don't know of a reason why we'd want > that in our code now. l20n basically solves the entire issue, as I understand > it. You mentioned that to me in our talk. But if that's not the case anymore I'm fine with it. That will give Mozmill, compared to other existing frameworks, the unique feature to run tests against localized builds.

(In reply to comment #21) > Adrian, can you also please update the documentation for that new function? Done ( https://developer.mozilla.org/en/Mozmill_Tests/Shared_Modules/UtilsAPI )

Mass move of Mozmill Test related project bugs to newly created components. You can filter out those emails by using "Mozmill-Tests-to-MozillaQA" as criteria.