188557 - Hard-coded application name "Navigator" in "Illegal plugin operation" error message

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Matthew Wilson]

User-Agent:       Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.3b) Gecko/20030110
Build Identifier: Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.3b) Gecko/20030110

When going to the specified URL, I get an error message "The plugin performed an
illegal operation. You are strongly advised to restart Navigator." I don't mind
getting an error - the page contains SVG and I don't think I do have an
up-to-date plugin - but the application name "Navigator" seems to be hard-coded.

Reproducible: Always

Steps to Reproduce:
1. Load the above URL without a suitable SVG plugin.

Actual Results:  
See an error "The plugin performed an illegal operation. You are strongly
advised to restart Navigator."

Expected Results:  
Should see "... You are strongly advised to restart Phoenix", if using Phoenix.
I don't know whether Mozilla should say "Navigator" or "Mozilla".

seems to be BadPluginMessage in
xpfe/components/xfer/resources/locale/en-US/downloadProgress.properties

Comment 1

[Henrik Gemal]

"plugin" is also spelled wrong. Should be "plug-in"

Comment 2

[Jonas Nordlund]

Just noticed this too in Firefox 1.0PR
Mozilla/5.0 (Windows; U; Windows NT 5.0; rv:1.7.3) Gecko/20040913 Firefox/0.10

And in this case, "plugins" aren't even called "plug-ins" anymore, but
"extensions". Would be best if this string wouldn't be hardcoded both to use the
proper product name and the term used in the product.

Comment 3

[timeless]

[e-mail]: a plug-in is a plug-in. it always has been, it always
will be. not to be confused with extensions.

Comment 4

[Jonas Nordlund]

Sorry, my bad, got confused :-)

Anyway, there's yet another reason to fix this since the Firefox Help won't find
any information about "plugins" or where to get them. It will, however, if you
search for "plug-ins".

Comment 5

[Stefan [:stefanh]]

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

Comment 6

[Phil Ringnalda (:philor)]

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

Comment 7

[Kevin Brosnan]

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

Comment 8

[Vidar Haarr (not reading bugmail)]

Neil: Could you advise on the following?

I guess we need to use the StringBundleOverride stuff here, but I'm not sure.

What I think is: when the StringBundle is initialized in <>, we need to add an
mOverrideBundle, which probably should be this one:
nsCOMPtr<nsIStringBundle> bundle;
bundleService->CreateBundle("chrome://global/locale/brand.properties",
getter_AddRefs(bundle));

And then add that as an argument when creating the StringBundle for
downloadProgress.properties.

The patch for the properties file would be as simple as this:
- BadPluginMessage=The plugin performed an illegal operation. You are strongly
advised to restart Navigator.
+ BadPluginMessage=The plugin performed an illegal operation. You are strongly
advised to restart %S.

Or am I completely wrong - does nsIPersistentProperties have anything to do with
this ?

Comment 9

[Vidar Haarr (not reading bugmail)]

Right, the '<>' was supposed to be filled with:
<http://lxr.mozilla.org/seamonkey/source/modules/plugin/base/src/nsPluginHostImpl.cpp#6082>

Comment 10

[Vidar Haarr (not reading bugmail)]

Attachment: C/C++ changes for nsPluginHostImpl

I was wrong in my last comment(s), as pointed to by bsmedberg and biesi on IRC.
Thankfully, they helped me along to the correct way.

These are the C changes. properties changes coming up.

Comment 11

[Vidar Haarr (not reading bugmail)]

Attachment: changes for downloadProgress.properties

Uses %S instead of 'Navigator'.

Comment 12

[Vidar Haarr (not reading bugmail)]

Attachment: includes the two above patches, and the second properties file

This includes all the three files in one attachment (sorry about the spam!). It
also includes the second (!) downloadProgress.properties (one for suite and one
for other apps).

The file should probably be moved, as biesi pointed out on IRC, but that's for
Peter to decide, I guess. It's also terribly confusingly named :)

Comment 13

[Johnny Stenback (:jst)]

Comment on attachment 168661 [details] [diff] [review]
includes the two above patches, and the second properties file

r+sr=jst

Comment 14

[Vidar Haarr (not reading bugmail)]

jst: As discussed on IRC, could you check this in when you get a chance?

Comment 15

[Vidar Haarr (not reading bugmail)]

Attachment: alternate approach

timeless suggested that we only use one bundle variable, so here's my try on
that approach

Comment 16

[Vidar Haarr (not reading bugmail)]

Comment on attachment 168997 [details] [diff] [review]
alternate approach

jst: timeless thinks this is a better approach - could you take a look?

Feel free to land the patch of your choice.

Comment 17

[Vidar Haarr (not reading bugmail)]

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

Comment 18

[Johnny Stenback (:jst)]

Comment on attachment 168997 [details] [diff] [review]
alternate approach

I'm fine with either patch here... r=jst

Comment 19

[akm]

Re: additional comment #1, shouldn't this text have "plugin" changed to
"plug-in" with a hyphen?

+BadPluginMessage=The plugin performed an illegal operation. You are strongly
advised to restart %S.

Comment 20

[Henrik Gemal]

yes. "plugin" should be "plug-in"

Comment 21

[Vidar Haarr (not reading bugmail)]

I'll update the patch. I think we'll go for the alternate approach in attachment
168997 [review], because it's smaller.
We'll wait till after 1.8a6 before landing it, though, as this isn't really
critical for an alpha anyway.

Comment 22

[Vidar Haarr (not reading bugmail)]

Attachment: plugin -> plug-in

New patch to that effect (derived from attachment 168997 [details] [diff] [review]).
Carrying forward r+sr=jst.

Comment 23

[[e-mail]]

Fix checked in.

Comment 24

[Walter K.]

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

Comment 25

[Olivier Cahagne]

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

Comment 26

[Vidar Haarr (not reading bugmail)]

Attachment: fix for legal chrome URL

One-liner fix for the chrome URL to brand.properties

Comment 27

[Robert Kaiser]

Comment on attachment 172802 [details] [diff] [review]
fix for legal chrome URL

r=me, as it concerns locale things and it's simple ;-)

Comment 28

[James Ross]

Comment on attachment 172802 [details] [diff] [review]
fix for legal chrome URL

I'm picking jst here because he has reviewed some earlier patches.

jst, this is a 1 line change to make the original fix actually work.

Comment 29

[Vidar Haarr (not reading bugmail)]

Attachment 172802 [details] [diff] was checked in by James - thanks!

Can anyone verify the fix?

Comment 30

[Matthias Versen [:Matti]]

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

Comment 31

[William Bumgarner [:zsinj]]

Another instance of this bug or a dupe? bug 300296

Comment 32

[Jo Hermans]

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

Comment 33

[Dave Townsend [:mossop]]

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