303398 - feedview should use DTDs for l10n, not properties

Status: RESOLVED INVALID

(Firefox Graveyard :: RSS Discovery and Preview, defect, P2)

Description

[Axel Hecht]

Using a properties file for l10n of Feedview is not required, but imposes
granting priviledges to the code, which are not necessary.
Using a DTD would also make the code easier, as it would involve less parameters
to the XSLT stylesheet.

Comment 1

[Asa Dotzler [:asa]]

->myk if we need to fix this.

Comment 2

[Asaf Romano (gone)]

Attachment: patch

Comment 3

[Anbo Motohiko]

(In reply to comment #2)
There are several unlocalizable strings in feedview.xsl.


Line 66: title attribute of link element.
>  66         <link rel="alternate" type="application/rss+xml" title="Current
Feed" href=""/>


Line 124~126:
> 124           <span id="timerspan">Refresh in <span id="timevalue">
> 125             <xsl:value-of select="$timerInterval"/>
> 126           </span> seconds</span>

Comment 4

[Asaf Romano (gone)]

Attachment: patch

more cleanup + removal of the timer feature.

Comment 5

[Axel Hecht]

Could you use the #if 0 kind of license plates everywhere?
That enables folks to use regular tools on these files.

Do we really need a properties file and a dtd? I think that an attribute value
template is acceptable for localizers.

Comment 6

[Asaf Romano (gone)]

#if 0 doesn't wfm n files which are not dtd files (for some reason, i don't see
why woudn't it work)

Comment 7

[Asaf Romano (gone)]

(In reply to comment #5)

> Do we really need a properties file and a dtd? I think that an attribute value
> template is acceptable for localizers.

Sounds kinda ugly to me. Does it really matter?

Comment 8

[Anbo Motohiko]

(In reply to comment #7)
How about this?

            <span id="number">
+             &articles.pre.label;
              <xsl:value-of select="$feedDescription"/>
+             &articles.post.label;
            </span>
 :
-  gFeedviewProcessor.setParameter(null, "feedDescription",
strbundle.getFormattedString("description", [len] ) );
+  gFeedviewProcessor.setParameter(null, "feedDescription", len );
 :
+<!ENTITY articles.pre.label  "This feed contains">
+<!ENTITY articles.post.label "articles.">

Comment 9

[Axel Hecht]

Dang, sorry. We can't use an AVT here in the first place, because this is
a span, so the content is not in an attribute at all.

I'm not that confident that split up strings are a better choice than a properties
file.

Regarding the license plates, it sure does work for XML files. You just have
to take care that the #if 0 is inside the language specific comment format.
Like you did for the DTD. Shouldn't be any problem in other file formats, too.

Comment 10

[Asaf Romano (gone)]

Axel, is this worth late-l10n or should we target this to 2.0?

(That said, I'm pretty sure 1.5-Feedview isn't done string-wise).

Comment 11

[Axel Hecht]

I have no preference really. Mostly, because I don't have an overview of what
feedview will look like for 1.5.

Comment 12

[Ben Goodger (use ben at mozilla dot org for email)]

Why does using a .properties file for feedview "grant privileges" to the code
executing in the content area? 

The use case is as follows:

browser.js:

var str1 = strings.getString("foo");
var str2 = strings.getString("bar");
xsltProcessor.setParameter("foo", str1);
xsltProcessor.setParameter("bar", str2);
var frag = xsltProcessor.transformToFragment(doc);

... aren't the strings str1/str2 just regular literals by the time the unsafe
content gets its grubby hands on it?

Comment 13

[Ben Goodger (use ben at mozilla dot org for email)]

What I mean to say is the strings in .properties are always used only by the
chrome code, never by the content/script loaded in the content area.

Comment 14

[Myk Melez [:myk] [@mykmelez]]

(In reply to comment #13)
> What I mean to say is the strings in .properties are always used only by the
> chrome code, never by the content/script loaded in the content area.

I think this bug is based on the expectation that we'll ultimately want to
implement feedview chrome as XBL and invoke it the same way we invoke XML
prettyprint.

Indeed, that's a better architecture (simpler, because we can eliminate hacks
like the reflection of data into XSLT parameters; more robust, because we can
hide our modifications to the DOM; and more powerful, because we can build
better interfaces with chrome XUL), but until someone implements a security
model for chrome bound to content, feedview chrome as XBL bound to the feed file
would have only content privileges, so it wouldn't have access to properties.

But we're not implementing such an architecture at the moment (the security
model needs to get sorted out first), so it shouldn't determine what we do for
localization of the current feature.

I note that the description of this bug starts "Using a properties file for l10n
of Feedview is not required", but feedview sets the document title and feed
description by first getFormattedString-ing the feedTitle and feedDescription
strings, then reflecting them into XSLT parameters.  As far as I can tell, DTDs
cannot provide this functionality, so this bug should be resolved invalid.

Comment 15

[Ben Goodger (use ben at mozilla dot org for email)]

I agree with what myk says about providing "enhanced" functionality within the
content area. It would be great if we could build widgets to be embedded in
untrusted content using XBL, and "wall" them off in a similar way to how the
file control is. 

Such a model is probably a long way off, and would require a good deal of
discovery, education, documentation etc on how to build such components "safely".

Comment 16

[Axel Hecht]

Regarding privs, I was talking about the priviledges required for the start-up
code. So it's not "feedview granting privs to something", but "the privs
feedview needs to run".
I don't really know the exact story, but there was apparently some issue with
how things used to work in some incarnation of this code. Yeah, fuzzy.
The argument was really about the privs that the feedview hook code needs. Not
a strong one.

I do think that property files are harder to translate, though, esp in this
context, as the UI string is miles away from the use of the l10n string.

Regarding the feasibility of using a DTD, it can be done in a XUL-based layout.
If the title ends up in a attribute value instead of a text-node, you can use
attribute value templates easily, which are not more cumbersome wrt l10n than
XSLT params.

Comment 17

[Phil Ringnalda (:philor)]

Feedview is (rather long) dead; long live Feed Preview.

Comment 18

[Tanner M. Young [:tmyoung]]

Resetting QA Contact to default.