56863 - make about:plugins localizable

Status: RESOLVED FIXED

(Core :: Internationalization, defect, P3)

Description

[Dawn Endico]

about:plugins should use string bundles to make the text localizable,
but can't because it doesn't have permission. bug 53493 addresses this.

Comment 1

[Ben Bucksch (:BenB)]

See bug 36908. It has a temporary, or at least partial, fix (with the
localizable strings as string vars defined at the top of the file). That bug
also has a long-term suggestion (anything rginda intended to do).

Comment 2

[Dawn Endico]

rginda thought that his component viewer was too techie for most
users of about:plugins and that combining them would be a bad idea.

Unless someone has a brilliant idea about how to improve the
functionality of about:plugins by turning it into a xul app,
I think its best to leave it as it is (html) and wait for
mstoltz's fix.

Comment 3

[ekrock's old account (dead)]

Not a Netscape 6 RTM blocker. FUTURE. This bug has been marked Future because
the Netscape engineer it is assigned to is overburdened.

Comment 4

[Peter Lubczynski]

How do you make something localizable?

Comment 5

[Andreas Becker]

Adding keyword l12y.

Comment 6

[Andreas Becker]

*** Bug 79209 has been marked as a duplicate of this bug. ***

Comment 7

[Karl Ove Hufthammer]

> How do you make something localizable?

It's easiest to turn it into XUL, and externalize all text to entities in a DTD 
file. As an added bonus, the about:plugins page will look much better.

Note: The bug blocking this has now been fixed.

Comment 8

[Peter Lubczynski]

Wow, XUL would be really nice but I don't have time to learn XUL. cc:ing some 
XUL folks to see if there are any extra cyles in their group to convert this 
HTML page to localizable XUL. 

Also, cc:ing SmartUpdate group as they may want to have about:plugins redirect 
to a server supplied smart web page for managing plugins.

Comment 9

[Dan Rosen]

Peter: Wrong XUL people. You want to CC XPApps (if anybody), not toolkit. CC'ing
vishy...

Comment 10

[Viswanath Ramachandran]

please nominate for nsbeta1 if needed. 

Comment 11

[Michele Carlson]

Ray - can you please comment on this bug.
thanks

Comment 12

[Peter Lubczynski]

Well, now that it's broken (since XPCDOM landing), maybe it's a good time to 
localize and/or move to XUL?

Can someone else take this bug? I don't know the first thing about making this 
localizable. It's just a plain HTML file now. IMHO, all of it should be taken 
care of by Netcenter and SmartUpdate. CC:ing selmer because I don't know who  
takes care of that. Perhaps have the pref for enabled SmartUpdate control if 
this pages redirects there or not?

Comment 13

[Dawn Endico]

>IMHO, all of it should be taken care of by Netcenter and SmartUpdate.

Ugh. what makes you say that? I hope you just mean that you think that
people at Netcenter are the ones who should do this work, and not that some
proprietary Netcenter feature should take the place of about:plugins.

Remeber, we're talking about Mozilla, not Netscape 6.

Comment 14

[Peter Lubczynski]

Well, then maybe perhaps only redirect for branded Netscape 6 or do my favorite 
thing in the world...just create another pref to specify the redirect URL. We 
still need something there for when smart update is diabled, the browser can't 
connect. Also, if they may not be ready for this, that's why it would be good to 
cc: the people working on that in this bug (or open another one).

Anyway, it should be localized either way.

Comment 15

[Rafael Ebron (:rebron)]

I agree with Dawn that about:plugins needs to stay and work and be localized for 
Mozilla and we do a redirect for a NSCP.com hosted plug-ins page.  

Todd, are you okay with this?  

Comment 16

[Peter Lubczynski]

I'm not the right owner for this. Sending over to localization. about:plugins is 
just a plain html file.

However, I am interested in having about:plugins redirect to Netcenter for  
branded "Netscape". Todd, please file a bug on me if you are interested in 
something like this.

Comment 17

[nhottanscp]

Reassign to rchen, cc to mcarlson.

Comment 18

[Peter Lubczynski]

Joe said he was interested in this.

Comment 19

[Frank Tang]

surely they are html with javascript. But the owner should make the string 
localizabel by store them into properties files or DTD files according to 
http://www.mozilla.org/projects/intl/string-resources.html as all other client 
side html/xul/js.

Comment 20

[Frank Tang]

ok, as exception, I will do this for you this time. But in general, the ui 
author in mozilla are responsable for localizability issue. 

Comment 21

[Frank Tang]

Attachment: make it localizable

Comment 22

[Peter Lubczynski]

Joe is on vacation in India so I'll take this.

Thanks for the patch Frank! Also, thanks for showing me how to do this in the
future. r=peterl

Comment 23

[Frank Tang]

peter, can you get someone to sr= this one and check in . Thanks.

Comment 24

[Johnny Stenback (:jst)]

sr=jst

Comment 25

[Christopher Blizzard (:blizzard)]

r=blizzard

Comment 26

[Jaime Rodriguez, Jr.]

Vishy - Let's get this one in for TM0.9.4. It looks baked, and has a r, and SR.
</8^). 

Thanks Ftang!

Comment 27

[Paul Chen]

nav triage team:

Marking target milestone 0.9.4 since it looks like it just needs to be checked in

Comment 28

[Peter Lubczynski]

Actually, I'm have some problems with this patch.  about:plugins doesn't work. I
get this error:

--** strBundleService failed: Permission denied to access property
JavaScript error:
jar:resource:///chrome/toolkit.jar!/content/global/plugins.html line 84: bundle
has no properties 

Comment 29

[Frank Tang]

forget the include the jar.mn change into the patch

Comment 30

[Frank Tang]

Attachment: missing jar.mn changes

Comment 31

[Peter Lubczynski]

Frank, it still does not work for me. Does it work for you? Perhaps I am just
applying it wrong but I did a whole clobber.

Comment 32

[Frank Tang]

yes, it does work for me before I submit it. I will double check. Maybe I foget
something in the patch.

Comment 33

[Alexey Chernyak]

Previously I've done work of converting about:plugins page to valid XHTML 1.0
Strict and warning-free java script.
That was done on the following file:
http://lxr.mozilla.org/mozilla/source/xpfe/global/resources/content/plugins.htmlHowever
I just found another file:
http://lxr.mozilla.org/mozilla/source/l10n/us/xp/aboutplg.html
which is the old HTML version of the same plugins page and was never modified.
Anyone knows what that is?
We probably should change it as well?

Comment 34

[Alexey Chernyak]

oh by the way before you check it in,
could you please correct a few trivial problems from bug 80676
like changing mimetype_label to be "MIME Type" instead of "Mime Type"

Comment 35

[Alexey Chernyak]

also lang="en" dir="ltr" attributes should be removed from <html> tag

I put them there when the question of localisation first appeared, hoping they
might be of some use.

But it appears to me with this patch they are useless now.
Having lang="en" in document which displays French text just doesn't make sense.

Comment 36

[Peter Lubczynski]

Reassign to ftang for check-in since it's working in his tree.

Since this missed the deadline for open checkin to mozilla0.9.4, I think it now
needs drivers approval.

Comment 37

[Andreas Becker]

Switching qa contact to ylong.

Comment 38

[Frank Tang]

ask driver approval. waiting for reply

Comment 39

[Asa Dotzler [:asa]]

a=asa on behalf of drivers.

Comment 40

[Frank Tang]

fixed and check in . 

Comment 41

[Frank Tang]

reopen, it seems it does not work now and show me a blank page.
If I type in chrome://content/global/plugins.html it will show me the correct
page. But if I use the menu Help:about plugins or type about:plugins, it won't work
It looks like some CAPs block our access to property file- could be related to
the patch in 86799 add [e-mail] to the cc list. 

Comment 42

[Frank Tang]

I back out my change to plugins.html and file a bug 98298 for the access issue.

Comment 43

[Frank Tang]

jbetak- can you help me to get this into m0.9.4 and trunk if 98298 got fixed. I 
back out the plugins.html from rev1.4 to the previous stated in both trunk and 
branch now (== rev 1.3) My code in rev1.4 cannot access string bundle due to 
security script checking for "about:plugins" . If they fix 98298, then please 
update -r 1.4 of your xpfe/global/resources/content/plugins.html and nmake -f 
makefile.win in xpfe/global it should show you the right plugins page. Then, 
please check in as rev 1.4 to fix this one.

Thanks.

Comment 44

[Jaime Rodriguez, Jr.]

This is a nice to have at this late date, and not a stop ship in my mind. But, I
could be wrong.

Msanz?

Comment 45

[msanz]

removing nsbranch+ and adding nsbranch- since it won't be fixed for this release

Comment 46

[msanz]

moving to 9.5

Comment 47

[[e-mail] (away - not reading bugmail)]

accepting, moving to M096

Comment 48

[Frank Tang]

move back to ftang

Comment 49

[Frank Tang]

assign

Comment 50

[nhottanscp]

move to 0.9.8

Comment 51

[Frank Tang]

mass move unimportant m9.8 bug to m9.9 for now. 

Comment 52

[Jaime Rodriguez, Jr.]

if this is "unimportant" than why are we working on it ... shouldn't we just
move it out to M1.0.1, so we can focus on higher priority issues? if this is
importnat, than we should nominate as nsbeta1 and plus it.

Comment 53

[Peter Lubczynski]

I think there is already a patch for this bug but it's blocked by bug 98298.

Comment 54

[Frank Tang]

shanjian- can you help to solve this issue.

Comment 55

[Frank Tang]

nsbeta1- per triage meeting

Comment 56

[timeless]

Attachment: make about:plugins localizable

mkaply indicates that his l10n team(s) have no trouble localizing html files,
so this patch should work for now. if people agree, a modified version of this
patch (fixing jar.mn to point to a locale/en-us disk directory) would be made.

endico is willing to do a cvs record copy of the plugins.html file to a
locale/en-us directory if we agree to this approach. doing that would preserve
cvs log/blame for the new location and allow the old file to be cvs removed (it
would still be available via dated, branched and revisioned checkouts)

Comment 57

[Peter Lubczynski]

Frank,

Insted of using XPConnectUniversal access to get to string bundles, could a
simple localized DTD be used here?

Comment 58

[Shanjian Li]

I like the patch presented by timeless in #56. rchen, what do you think? Or
should we ask some other l10n experts? 

reassign to timeless. (timeless, if your approach is denied and you don't want
to own this bug any more, you can reassign it to me. )

Comment 59

[timeless]

ok, now to ask endico or leaf for help :)

Comment 60

[Mike Kaply [:mkaply]]

When I said "no trouble with HTML", I meant "normal" HTML.

In looking at about:plugins, the document.writeln stuff tends to bother 
translation people.

What our translation people prefer (I know it is ugly) is to put JS variables at 
the top of the HTML file to group the translated stuff.

Personally, I would prefer that or the string bundle stuff. 

Having the translated stuff mishmashed into the document.writelns is really 
ugly.

Comment 61

[timeless]

i suppose we could make a plugin object and have a custom .prototype.toString method.  Then the localizers would basically rewrite that one method.

Comment 62

[Robert Kaiser]

Attachment: new version of ftang's patch

I cleaned up ftang's patch a bit, using less variables and using the already
checked in plugins.properties file.

I think it's better to use less variables there...

Comment 63

[Robert Kaiser]

Attachment: second version of my patch, orginally derived from ftang's approach

Well, here's a patch that works with current builds.

It deals with the fact that plugins.html has moved to communicator and that is
has changed quite a bit due to my last patch.

The patch also enables privileges for about:plugins in nsAboutRedirector.cpp -
we should not need this, but because of bug 98298 we still don't have
stringbundle acces the way we want it, so we have to get full chrome access to
try the patch.

Comment 64

[Alexey Chernyak]

CCing Jesse to check security implications of this patch.

+  document.writeln("<title>" + pluginsbundle.GetStringFromName("title_label") +
"</title>");

should be "<\/title>"

This is now an invalid HTML document because it is missing a <title> tag.
It should be valid both, prior and after execution of any scripts.

Comment 65

[Robert Kaiser]

Alexey, thanks. I'll correct the "<\/title>" issue in my local tree for now,
I'll attach a new patch when I hear some words about security...

Comment 66

[Frédéric Kayser]

Has this file been moved into the en-US.jar alongside about.html in
/chrome/en-US/locale/en-US/global/ ?

Comment 67

[Robert Kaiser]

Frédéric:
This has nothing to do with about.html - and plugins.html has been moved from
global to communicator for bug 131477

Comment 68

[Robert Kaiser]

Attachment: patch v3: re-sync with trunk

As there was a section about plugindoc.mozdev.org added recently, this new
patch (v3) adds support for that as well.

Comment 69

[Ben Bucksch (:BenB)]

If you're parametrizing so much, it would be a good idea IMHO to also
parametrize the URLs.

Comment 70

[Mitchell Stoltz (not reading bugmail)]

If you're going to make about:plugins be a chrome page again, could we add some
HTML escaping to all of the strings written to the page? Otherwise there's a
possibility of some nasty security holes.

Comment 71

[Christian :Biesinger (don't email me, ping me on IRC)]

mstoltz: an alternative to that would be to make about:plugins generated from
C++ code - it would need neither javascript nor chrome privileges then.

Comment 72

[Mike Kaply [:mkaply]]

Just to reiterate something I have said in the paste.

Some plugins actually create instances of themselves in the about:plugins page
that allows you to access settings to their plugin using some special mode (like
that displays a button)

So the about:plugins must provide full HTML support.

Comment 73

[Robert Kaiser]

mkaply:
Do I get this right - we can't even escape the plug-in strings as mstoltz
suggests in Comment #70 because that would stop some plug-ins' features from
working?

Comment 74

[Mike Kaply [:mkaply]]

That is correct.

Here is what the OS/2 Shockwave Flash has for its description. I know there are
Windows plugins that at least use the description to have a link to their website.

Shockwave Flash 5.0 r67<br><embed aboutmode=yes
type=application/x-shockwave-flash width=170 height=34></embed>&#169;2000-2002 <
a href=http://www.innotek.de>Innotek&#174; Systemberatung GmbH</a>

this is why about:plugins has to be REAL HTML.

Because people have been able to put HTML in their Description in the past so
that it becomes live in about:plugins.

Comment 75

[Robert Kaiser]

Attachment: patch v4: localize URLs as well

OK, here's a patch that addresses what Ben Bucksch has said. It's correct that
the URLs should be localizeable as well, they should go to the content pack
though, as all other URLs are there as well. As the labels of the links state
the neames of their domains, they should always go with those URLs, so I also
moved them over to the content pack.

The security issue is still open though. mkaply, thanks for clarifying that we
need real HTML here.
mstoltz - or anyone else - can you come up with a solution for the security
issue? Is there a chance to fix the original issue why bug 98298 was filed? Can
you come up with a way to use stringbundles without giving us full chrome
rights here?
I must admit that I'm not happy with those full privileges either but I already
mentioned in comment #63 that we currently need it to be at least able to try
the patch. I would be much happier (I guess we all would) if bug 98298 would be
fixed in a "correct" way and we wouldn't have to work around it.

Comment 76

[Mitchell Stoltz (not reading bugmail)]

I think I have a solution for bug 98298. I don't think the risk is high enough
to stop you from checking in the above patch as is, if it's urgently needed.
I'll try to fix the privileges issue ASAP.

Comment 77

[Robert Kaiser]

Attachment: patch v5: include jar.mn files in the patch

biesi noted that I forgot the jar.mn changes in the patch. Here is a new one
that includes those changes as well. Thanks!

Comment 78

[Robert Kaiser]

I think this should go into the 1.3 beta.

No matter if we go the path of bug 98298 or bug 186490, we will be able to use
the .properties file, and it will help L10n quite a bit to have this working.

Comment 79

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 110660 [details] [diff] [review]
patch v5: include jar.mn files in the patch

ok, you can have r=biesi if my patch for about:plugins in C++ does not get
reviews before 1.3b

Comment 80

[Robert Kaiser]

BTW, taking bug as it's my patch.

Comment 81

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 110660 [details] [diff] [review]
patch v5: include jar.mn files in the patch

Please add some comments in plguins.properties saying that those strings will
get document.write() called on them and hence should escape any HTML special
chars...

(and could we move to createTextNode() to avoid that problem, pretty-please??)

Comment 82

[Robert Kaiser]

Checking in mozilla/xpfe/global/jar.mn;
/cvsroot/mozilla/xpfe/global/jar.mn,v  <--  jar.mn
new revision: 1.63; previous revision: 1.62
done
RCS file:
/cvsroot/mozilla/xpfe/communicator/resources/locale/en-US/plugins.properties,v
done
Checking in mozilla/xpfe/communicator/resources/locale/en-US/plugins.properties;
/cvsroot/mozilla/xpfe/communicator/resources/locale/en-US/plugins.properties,v 
<--  plugins.properties
initial revision: 1.1
done
Checking in mozilla/xpfe/communicator/resources/locale/en-US/region.properties;
/cvsroot/mozilla/xpfe/communicator/resources/locale/en-US/region.properties,v 
<--  region.properties
new revision: 1.5; previous revision: 1.4
done
Checking in mozilla/xpfe/communicator/resources/content/plugins.html;
/cvsroot/mozilla/xpfe/communicator/resources/content/plugins.html,v  <-- 
plugins.html
new revision: 1.11; previous revision: 1.10
done
Checking in mozilla/xpfe/communicator/jar.mn;
/cvsroot/mozilla/xpfe/communicator/jar.mn,v  <--  jar.mn
new revision: 1.26; previous revision: 1.25
done
Checking in mozilla/netwerk/protocol/about/src/nsAboutRedirector.cpp;
/cvsroot/mozilla/netwerk/protocol/about/src/nsAboutRedirector.cpp,v  <-- 
nsAboutRedirector.cpp
new revision: 1.14; previous revision: 1.13
done
Removing mozilla/xpfe/global/resources/locale/en-US/plugins.properties;
/cvsroot/mozilla/xpfe/global/resources/locale/en-US/plugins.properties,v  <-- 
plugins.properties
new revision: delete; previous revision: 1.1
done


OK, fixed. However, this now leaves bug 98298 as a security issue.
Anyway, the long-term solution for this (and for failing when JS is deativated)
is bug 186490 - but this won't happen before the tree opens for 1.4 Alpha, so
this bug is the way to go for 1.3 - Ideally, we can fix the (theoretical)
security issue in the time until 1.3

bz: I added a L10n note in plugins.properties about escaping, thanks!