243387 - about:plugins broken by download size reduction changes

Status: VERIFIED FIXED

(Firefox :: General, defect, P2)

Description

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

my pruning of the communicator package removed the about:plugins page. Need to
add it back.

Comment 1

[Steffen Wilberg]

Attachment: patch

We need plugins.html, plugins.properties and region.properties in
communicator-region; in order to register the latter, we need the appropriate
contents.rdf as well.
The first change is just whitespace.

Comment 2

[Steffen Wilberg]

Ben meant Bug 243091 by the way.

Comment 3

[Alexey Chernyak]

Make sure other about pages are not lost like about:, about:mozilla and
about:config.

Comment 4

[Mike Connor [:mconnor]]

those three have been forked for a while, so no worries there

Comment 5

[Steffen Wilberg]

about:blank, about:buildconfig, about:cache, about:cache-entry (needs arguments)
and about:credits work as well.
about:about doesn't work because bug 220253 has been marked wontfix.
The only other broken page is about:logo, but that doesn't work since ages.

Comment 6

[Steffen Wilberg]

Attachment: fix typeaheadfind as well

This fixes bug 243386 (find as you type aka typeaheadfind) as well.
Suggestion by Pike (bug 243386 comment 4): We've got to register
navigator/locale.

Because:
http://lxr.mozilla.org/mozilla/source/extensions/typeaheadfind/src/nsTypeAheadFind.h#71

 70 #define TYPEAHEADFIND_BUNDLE_URL \
 71	    "chrome://navigator/locale/typeaheadfind.properties"

Comment 7

[Steffen Wilberg]

Comment on attachment 148501 [details] [diff] [review]
fix typeaheadfind as well

Mike, I guess you can review it somewhat faster than Ben...
See comment 1 and comment 6 for explanation.

Comment 8

[Mike Connor [:mconnor]]

Comment on attachment 148501 [details] [diff] [review]
fix typeaheadfind as well

did these changes land on the branch yet?

Comment 9

[Steffen Wilberg]

> did these changes land on the branch yet?
Yes, they were part of Ben's checkin from 2004-05-16 11:16:
http://bonsai.mozilla.org/cvsgraph.cgi?file=mozilla/browser/extensions/package-fixup/jar.mn

Comment 10

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

NO! this is the wrong fix. Please back it out of the trunk and branch!!!!

Comment 11

[Steffen Wilberg]

> NO! this is the wrong fix. Please back it out of the trunk and branch!!!!
It wasn't checked in yet, neither trunk nor branch.
Can you explain what's wrong with the fix, please?

Comment 12

[Steffen Wilberg]

Comment on attachment 148501 [details] [diff] [review]
fix typeaheadfind as well

All right, I just saw your patch in bug 243386. So this is obsolete.

Comment 13

[Steffen Wilberg]

Removing dependency to bug 243386.

Comment 14

[Steffen Wilberg]

Comment on attachment 148351 [details] [diff] [review]
patch

Ben, is this correct? Or has the fix to be something like you did in bug
243386?
Explanation is in comment 1.

Comment 15

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

Attachment: better patch

Make about:plugins be packaged into global, not communicator.

Comment 16

[Mike Connor [:mconnor]]

this didn't make sense to me the first time I read it, it still seems wrong, I
don't remember thinking this was OK at any point.

Maybe I set the flag on the wrong bug?  in any case, mea culpa

Comment 17

[Steffen Wilberg]

Comment on attachment 148983 [details] [diff] [review]
better patch

>Index: xpfe/global/resources/content/plugins.html
>===================================================================
>+  var pluginsbundle = strBundleService.createBundle("chrome://communicator/locale/plugins.properties");
>+  var regionbundle = strBundleService.createBundle("chrome://communicator-region/locale/region.properties");
>+
>+  document.writeln("<title>" + pluginsbundle.GetStringFromName("title_label") + "<\/title>");
>+</script>
>+<link rel="stylesheet" type="text/css" href="chrome://communicator/skin/plugins.css">

This doesn't work as long as plugins.properties and region.properties don't
exist. plugins.css would be helpful as well.

Comment 18

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

Attachment: patch, part 2

This is also required, in addition to "better patch"

Mike, can I get a quick review from this so I can check all this in on the
trunk?

Comment 19

[Mike Connor [:mconnor]]

Comment on attachment 149309 [details] [diff] [review]
patch, part 2

r=[e-mail]

Comment 20

[Tim Meader]

This seems fixed to me.  Close?

Comment 21

[Mike Connor [:mconnor]]

its not fixed on trunk yet

Comment 22

[Daniel Luz]

I believe at least the blocking0.9+ could be removed then

Comment 23

[amano]

Why didn't make the patch it onto the trunk, as comment #18 suggested? Or fell
it off the radar?

Comment 24

[Mike Connor [:mconnor]]

the patch needs SR to land on trunk.

Comment 25

[Tim Meader]

Just an FYI, there seems to be an issue with the about:plugins fix, on AVIARY.
It seems that something has changed with the about:plugins that causes all
themes other than the default theme to not use the stylesheets. The page shows
up as completely plaintext.

In addition, if you switch to a new theme without restarting the browser, and
then type in about:plugins and hit return... Firefox crashes instantly  :(

I can open a new bug for the latter if you want, but the first one seems like a
problem that perhaps this patch could be changed to accomadate? Or if it's
simply a new .css file, then perhaps put out a notice to all the theme authors
somewhere?

Comment 26

[Tim Meader]

Forgot to mention, sorry, I'm using the Official 0.9 build (installer version).

Comment 27

[Steffen Wilberg]

Yes, theme authors need to fix their themes. I already made a post in the themes
forum: http://forums.mozillazine.org/viewtopic.php?t=85200

I can't reproduce the crash with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US;
rv:1.7) Gecko/20040616 Firefox/0.9 (Steffen).

Comment 28

[Tim Meader]

Could be fixed in the past couple days.

I'll give a build today or tomorrow a spin and let you know. In the meantime,
anyone else who's using the Official, can you confirm it?

You have to make sure to hit "Use Theme" then just close the Theme Manager, and
DON'T restart Firefox before going to about:plugins.

Comment 29

[Tim Meader]

Okay, it's not with every theme. Doodle (Classic) is one that does it ALWAYS
though.  As well as Firefox Modern from what I can tell.

There's another anomally too with themes that I've noticed, but that's not
related to about:plugins at all, so that's a new bug.

If you could try downloading and using Doodle (Classic) though and see if you
can reproduce the error, that would be cool.

Comment 30

[Colin J Thomson]

just tested with the latest trunk (not the installer version):
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8a2) Gecko/20040616 Firefox/0.8.0+
with a fresh .firefox DIR no extensions/themes and about:plugins still does not
work.
HTH

Comment 31

[Steffen Wilberg]

Tim: sorry, no crash with that theme as well.
Colin: that's why this bug isn't marked as fixed. It's only fixed on the branch,
which includes FF 0.9, but not on the trunk. See comments 20, 21, 23, 24.

Please file new bugs if you have any issues other than about:plugins not working
in trunk builds (that's what this bug is about yet), or being unstyled in branch
builds with non-default themes (theme authors need to fix this).

Comment 32

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

Thanks Mike, I'll check this in later on this afternoon. 

Comment 33

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

Fixed on trunk, too. 

Comment 34

[Nobu Nari]

Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8a2) Gecko/20040620 firefox 0.8.0+

'about:plugins' works fine.   Thanks!

Comment 35

[Simon Paquet [:sipaq]]

Verified fixed 2004-06-21