131477 - Improve design of about:plugins page

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, enhancement, P5)

Description

[mozilla]

The current page looks very rudimentary, as if coded in early generation HTML.

I have taken the liberty of creating a replacement page that has new CSS code. I
have attached that to this bug report. Although the final page belongs in a jar
file, this will work as a standalone web page. To really appreciate the change,
you should install a number of plugins including plugins with multiple MIME
types like Quicktime. You can download a package of plugins from
http://www.shepherdstown.com/other/mozilla.

Since the new page includes color, I tried to use a scheme that works with both
Modern and Classic without appearing monochromatic. I have attached that color
scheme as a gif image for comparison. If the consensus is for a different color
scheme, the colors on this page can be replaced in less than a minute.

If people like this page, I can also improve the L&F of the about: page.

Comment 1

[mozilla]

Attachment: HTML code

Comment 2

[mozilla]

Attachment: Color palette used

Comment 3

[Alex Bishop]

Marking NEW, changing severity to enhancement, resummarizing (from "Improve L&F
of about:plugins page" to "Improve design of about:plugins page".

Comment 4

[Peter Lubczynski]

Nice page!

1. npqtplugin3.dll is extending the width of that table. Allow the rows to wrap.

2. Would it be possible to use the color scheme of the current skin? I've seen
it done in nsEngineer by importing the chrome://communicator/skin/ stylesheet.

3. Netscape.com has a new similar page:
http://home.netscape.com/plugins/manager.html
Maybe provide a link?

The actual document lies here:
http://lxr.mozilla.org/mozilla/source/xpfe/global/resources/content/plugins.html

BONUS: This page needs to be localizable. See bug 56863.

Comment 5

[rubydoo123]

The update looks great, and I agree with all of the points Peter has made. Also,
is there a possibility that we could add a new column, maybe titled Status. The
status column could have two possible outcomes: default[date] or vendor[date],
Where default notes that this plugin came with the installation, vendor notes
that it is either an upgrade to a default or the user went to a vendor site and
downloaded the plug-in, and date is the date the plug-in was installed.

Comment 6

[Ian Pottinger]

adding self to cc list

Comment 7

[Jeremy M. Dolan]

The font and size of the HTML page is unreadable for me on linux. I'm also a bit
freaked a page served by http has access to know my file structure

/usr/java/plugin/i386/ns610/libjavaplugin_oji140.so
/opt/mozilla/2002-04-15/plugins/libnullplugin.so
/home/mozilla/.mozilla/plugins/rpnp.so
etc

Can the new page be made to enable/disable plugins? The built in java-only pref
we currently have needs to go. I desperately crave a runtime flash toggle. I
want it off for 90% of flash pages where it's just ads or useless eyecandy.

It would have been nice if each plugin could provide a link to a vendor page
about it...not sure if the plugin API could be extended to do this, but it seems
like the new page is missing some place to click to find out more about
mysterious plugin XYZ you might see there.

Comment 8

[Alex Bishop]

> Can the new page be made to enable/disable plugins?

That's bug 19118. I think this bug should stick to just a cosmetic redesign.

> It would have been nice if each plugin could provide a link to a vendor page
> about it...not sure if the plugin API could be extended to do this

The QuickTime plug-in on Windows contains a link to the QuickTime website (it's
in the description) so I guess this is something that we have to rely on the
vendors to provide.

Comment 9

[mozilla]

I don't think the problem is with the font size but, rather, the insufficient
fonts included with major distributions of linux (apparently including your
installation, Jeremy). See bug 46415 and the release notes for 1.0.0.

Changing the font size to make it larger for those with poor quality fonts is
not the solution because this will result in oversized text on systems with high
quality fonts and, thereby, reduce the space available to display plug-in
information. 

I would argue that including 5+ base fonts (1 for each category) with all
distributions of Mozilla is a good thing (ensuring consistency of page rendering
across all platforms). Clearly, that is outside the scope of this enhancement
request.

All my proposed patch does is add CSS enhancements to the existing html page. It
doesn't alter how the current page works.

Comment 10

[rubydoo123]

So, you prefer not to add the additional column? Should I open a new bug for 
that request?

Comment 11

[Jeremy M. Dolan]

Is there any case where enabled=no? Even with Java disabled in the prefs, the
page still says it's enabled. Should we ditch this column for now until there's
a way to disable them, so people aren't looking around for something that
doesn't exsist?

Comment 12

[Peter Lubczynski]

All the info on this page comes from the DOM. See Nav 4.x JS guide as to how to
use  'enabled'

Comment 13

[rubydoo123]

what is the status of this work? Should this be reviewed for check-in, should we
include work to capture the added requests?

Comment 14

[Robert Kaiser]

I'm doing a patch for this.
I now did a version of the page working off the current about:plugins page and
making it themeable. I have a style sheet working to style it the way we have it
now, and a style sheet to incorporate the HTML work in this bug's first
attachment. My current version also incorporates bug 56863 but there we have bug
98298 still blocking that feature.

I'll do a patch here with the "old-style" layout in the classic theme and the
new-style layout in the modern theme, just to show how configurable that work is.

Comment 15

[Robert Kaiser]

Attachment: patch to make about:plugins localizeable, first try

this patch make plugins.html themeable, and adds stylesheets for classic and
modern themes.
Currently, classic version looks like the current version, and modern
incorporates the proposed design of attachment 1 [details] [diff] [review].

The patch also shifts a bit of the string code but leaves actual strings
unchanged (hardcoded in the file) because we currently can't make them
localizeable (see that other bug mentioned before). That can be also changed
more easily when this patch is applied.

Comment 16

[Robert Kaiser]

Taking and retargetting.

Can someone review this stuff?
I'm sure it will need some work like making classic version more beautiful as
well, and probably changing colors of modern version to better fit the theme -
but I'd really like someone to look at this and tell me further directions...

Comment 17

[Robert Kaiser]

Attachment: patch to make about:plugins localizeable, first try (updated)

OK, plugins.html was changed in trunk, so I had conflicts with my patch, this
one is updated to current trunk to apply cleanly.

Comment 18

[Robert Kaiser]

Attachment: make about:plugins themeable, and fit-in new style sheet with classic

Comment 19

[Robert Kaiser]

OK, this new patch does style classic better than the last one. Now classic gets
a new look as well, using all system colors, as we are used to whenusing the
classic theme.
The 'old' look of about:plugins is completely gone now, though the style sheet I
used in earlier patches still applies to get the 'old' look if anyone wants it.
I really hope someone will try this patch, I really want to get this in, at
least for 1.2alpha.

Comment 20

[Robert Kaiser]

Attachment: screen shot of Classic theme about:plugins page

OK, per a request of peterl, I made a screen shot of Classic theme with my new
about:plugins page.
I don't attach a screen shot of Modern now, because currently this would be the
same as if you just load the first attachment of this bug, and I think the
final version here should match the corlor scheme of Modern a bit better. I'll
work on this next.

The Classic version looks quite finished to me and I want to leave it as is,
I'm open for further changes though.

Comment 21

[rubydoo123]

looking at the patch:
HTML section
1. Could you use margin-top or padding-top on the HR instead of using the empty
paragraph?
2. When listing the filename, why are you placing an empty dd? Could you apply
the margin or padding instead?
3. If you are following strict, you need the TBODY element
4. After closing the TBODY and TABLE, you need to close the two opened DIV elements
5. I may be missing it, but I did not see the closing BODY and HTML elements

CSS section looks great

Comment 22

[Robert Kaiser]

Attachment: screen shot of Modern theme about:plugins page

And again I did some work here. This is a screen shot of the new Modern themed
about:plugins page. Taken with patch v3.

Comment 23

[Robert Kaiser]

Attachment: patch v3: make about:plugins themeable, with fitting CSS for Classic & Modern themes

This is patch v3, ready for review now.

The patch has style sheets that perfectly blend in with Classic and Modern
themes, as seen in the screen shots.
I also inserted a <tbody> per beppe's request No.3

Comment 24

[Robert Kaiser]

Attachment: plugins.html like it will look after patch v3 is applied

beppe:
To address your Comment #21 - This attachment is the version of plugins.html
how it will look after my attachment.
Addressing your points one by one:
1. The empty paragraph is in the current file, my patch removes it.
2. It's not <dd></dd>, it's thw other way round, my patch has </dd><dd>.
   I'm closing one dd, and opening another one.
3. I added the <tbody> now, thanks.
4. I'm closing all open elements, it's only one div that's open until the end
   of the document.
5. Like the div, they were already closed in the original file, and therefore
   that's not listed in the patch. See the new attached file, should be better
   to read.

Comment 25

[rubydoo123]

cool -- thanks! I should have looked at the entire file. it looks good to me

Comment 26

[Alex Bishop]

Comment on attachment 91840 [details] [diff] [review]
patch v3: make about:plugins themeable, with fitting CSS for Classic & Modern themes

>+var netcenter_label = "Netscape Netcenter";

It's not called Netcenter any more; it's Netscape.com.

Comment 27

[Robert Kaiser]

beppe:
does that count as r= from you?

Alex:
correct, I just took that from current implementation but I'll correct it
(either with a new patch if other comments are coming in or with final checkin),
thanks.

Comment 28

[rubydoo123]

yep, r=beppe

Comment 29

[Robert Kaiser]

Comment on attachment 91840 [details] [diff] [review]
patch v3: make about:plugins themeable, with fitting CSS for Classic & Modern themes

marking r= per beppe's last comment.

Comment 30

[Robert Kaiser]

Attachment: patch v4: make about:plugins themeable, address review nits

Patch v4 addresses some review nits from different people:
- Change Netscape Netcenter to Netscape.com per Alex Bishop
- Don't include <th> in <tbody> but in <thead> per nick (on IRC)
- Better indentation in plugins.html per doron (on IRC)

Comment 31

[Robert Kaiser]

Comment on attachment 92300 [details] [diff] [review]
patch v4: make about:plugins themeable, address review nits

carrying over review

Comment 32

[rubydoo123]

marked bugscape bug 5839 against this bug. The issue within that bug is as follows:
Redirect About:Plugins

We must ensure that the appropriate redirect is handled properly to display the
About:Plugins page

Comment 33

[Robert Kaiser]

beppe:

I don't understand what you mean with your last comment, can you describe that
more detailed, please?

Additionally, do you have any contact to beard? I asked him for sr on this
bug/patch but got no reaction so far...

Comment 34

[rubydoo123]

Hi Robert: Sorry, bugscape is an internal bug tracking database. The note about
the redirect has to do with stuff that we need to verify once you get your
updated patch checked in. We need to verify that when users select to go get an
update, that we direct them through the process.

Patrick is on sabbatical and will not be back for a month or so.

Comment 35

[Joe Hewitt (gone)]

Comment on attachment 92300 [details] [diff] [review]
patch v4: make about:plugins themeable, address review nits

Not so sure I like the idea of plugins.css being under chrome://global.  I'd
like to think of global as being reserved for XUL chrome.  Perhaps there should
be another directory under themes for embedded content

Comment 36

[rubydoo123]

I asked Joe to give the sr=, having a subdir sounds like a good idea.

Comment 37

[rubydoo123]

I should ask this here: Joe -- would you folks be the one to create that dir? If
you can, then Robert can modify the file to point to the correct location

Comment 38

[Robert Kaiser]

hewitt:
That rises the question if plugins.html should be in chrome://global as well...
I don't think it's good to have plugins.html and plugins.css in different chrome
components, that's why I placed this in global.
It's alright with me to move plugins.css in some other dir, I just think we
shouldn't place the two files in different chrome "components"...

Comment 39

[Joe Hewitt (gone)]

This really doesn't belong in global, but now that I think about it, I could
deal it it being in chrome://communicator

Comment 40

[Robert Kaiser]

Attachment: patch v5: make about:plugins themeable, move it to communicator

Joe, thanks for looking into that.

I now have a new patch that also moves plugins.html to communicator. I also
have to touch nsAboutRedirector now to point it to communicator now.

beppe, hewitt, can I get r/sr on this from you?

Comment 41

[rubydoo123]

looking at

in RCS file: mozilla/xpfe/communicator/resources/content/plugins.html
need to change:
+      document.writeln("</thead>"); to
       document.writeln("<\/thead>");

What is this used for:
+
+th + th,
+td + td {
+  border-left: 1px dotted ThreeDShadow; 
+}

are you defining a nested th?

Comment 42

[Robert Kaiser]

> need to change:
> +      document.writeln("</thead>"); to
>        document.writeln("<\/thead>");

thanks, done in my local copy.

> What is this used for:
> +
> +th + th,
> +td + td {
> +  border-left: 1px dotted ThreeDShadow; 
> +}
>
> are you defining a nested th?

No, the CSS '+' ('adjacent sibling') selector allows me here to only apply that
left line to a cell that has another cell right before it, i.e. all cells but
the leftmost cell. That way, the border is only drawn between cells but not on
the left border of the table.

see also: http://www.w3.org/TR/REC-CSS2/selector.html#adjacent-selectors

Comment 43

[rubydoo123]

great -- I didn't recognize that selector choice.

r=beppe

Comment 44

[Robert Kaiser]

Joe Hewitt, could you please sr= the new patch?

Comment 45

[Joe Hewitt (gone)]

Comment on attachment 98132 [details] [diff] [review]
patch v5: make about:plugins themeable, move it to communicator

sr=hewitt

Comment 46

[Robert Kaiser]

thanks hewitt.

leaf:
could you please copy the revision history for
mozilla/xpfe/global/resources/content/plugins.html
to
mozilla/xpfe/communicator/resources/content/plugins.html
so we won't lose historical info with that change?
Thanks!

Comment 47

[Dawn Endico]

i've copied plugins.html over to its new location. In the checkin comment
to the new file, please mention where it was copied from.

Comment 48

[Robert Kaiser]

thanks endico and all who helped to get this fixed!

Checking in xpfe/communicator/resources/content/plugins.html;
/cvsroot/mozilla/xpfe/communicator/resources/content/plugins.html,v  <-- 
plugins.html
new revision: 1.8; previous revision: 1.7
done
Checking in xpfe/communicator/jar.mn;
/cvsroot/mozilla/xpfe/communicator/jar.mn,v  <--  jar.mn
new revision: 1.22; previous revision: 1.21
done
Checking in themes/modern/communicator/plugins.css;
/cvsroot/mozilla/themes/modern/communicator/plugins.css,v  <--  plugins.css
initial revision: 1.1
done
Checking in themes/classic/communicator/plugins.css;
/cvsroot/mozilla/themes/classic/communicator/plugins.css,v  <--  plugins.css
initial revision: 1.1
done
Checking in themes/modern/jar.mn;
/cvsroot/mozilla/themes/modern/jar.mn,v  <--  jar.mn
new revision: 1.115; previous revision: 1.114
done
Checking in themes/classic/jar.mn;
/cvsroot/mozilla/themes/classic/jar.mn,v  <--  jar.mn
new revision: 1.100; previous revision: 1.99
done
Checking in netwerk/protocol/about/src/nsAboutRedirector.cpp;
/cvsroot/mozilla/netwerk/protocol/about/src/nsAboutRedirector.cpp,v  <-- 
nsAboutRedirector.cpp
new revision: 1.11; previous revision: 1.10
done
Checking in xpfe/global/jar.mn;
/cvsroot/mozilla/xpfe/global/jar.mn,v  <--  jar.mn
new revision: 1.59; previous revision: 1.58
done
Removing xpfe/global/resources/content/plugins.html;
/cvsroot/mozilla/xpfe/global/resources/content/plugins.html,v  <--  plugins.html
new revision: delete; previous revision: 1.7
done

should be FIXED.

(btw, I changed the <link rel="stylesheet" ...> to point to the correct .css and
added class=\"enabled\" for the th of the enabled column for better themability
in the version I checked in.)

Comment 49

[mozilla]

Attachment: About.xhtml is the About Mozilla page

It might be wiser to open a new bug for this but the changes are minor and
relate to this bug (and would undoubtedly involve the same people)...

This attachment improves the look of the About Mozilla page by copying the look
and feel (and links to the stylesheet) of the plugins page. Thus, the new About
Mozilla page is also themeable.

Ultimately, I would use a different image since I feel the current 'star'
graphic is rather unwieldy. Perhaps something relatively short in height, wide
in width and rectangular in shape. But that is work for another day...

Comment 50

[Robert Kaiser]

[e-mail]:
please don't clutter this bug with a different issue, file a new one for about:
if you want that to be themeable.
Also, create a new .css for about:, don't use tables where no tables needed, and
never use style="..." in a chrome file, that is in fact the opposite of making
it themeable!

Comment 51

[shrirang khanzode]

'about:plugins' design has improved...

Comment 52

[Simon Fraser [no longer active]]

This change caused about:plugins to break for embedders, since they don't get
the CSS file by default. Please fix the package files.

Comment 53

[Simon Fraser [no longer active]]

Er, please fix the embedding manifest files in embedding/config.

Comment 54

[Alfred Kayser]

Attachment: Another version of plugins.css for classic (and modern)

This stylesheet shows how the layout/presentation of the
about:plugin page can be much nicer.
To see even better examples, checkout the NautiPolis 1.4
and Walnut 1.4 themes.
In light of the redesign of mozilla.org, also the about: pages
need some refreshment.

Comment 55

[Robert Kaiser]

Alfred:
If you want additional changes to the CSS, please file a different bug for that.
This bug is considered FIXED, the only changes why this remains reopened are the
embedding changes which I try to fix here, and everything else should go into
new bugs.

Comment 56

[Robert Kaiser]

Attachment: patcht trying to fix embed.jar

OK, here's a patch trying to fix embed.jar - also including the files
introduced by bug 56863

I'm not sure if region.properties ends up correctly there or if we need
additional tweaks for embed to get the URLs from that files...

(additionally, obsoleting all attachments which are old and now unneeded or
don't belong here)

Comment 57

[David Haas (gone, not reading mail)]

Robert:

As mentioned in bug 199203, region.properties is NOT ending up in the embed.jar,
even with your patch.  And a brute copy & paste of
US/locale/US/communicator-region/(contents.rdf and region.properties) into the
embed.jar (at local/US/communicator-region/*), the javascript plugins.html still
can't find region.properties.

Is it the offset directory structure?  Any chance you have time to take another
look at this?

Comment 58

[Robert Kaiser]

The original bug here was fixed a long time ago.

With recent move of plugins.html back to global/ (checked into trunk by firefox
dictator without seamonkey sr, btw), sitauation for embed has changed again. if
there is still an issue, please open a bug, assing it to ben goodger (as he
seems to care about what plugins.html is doing now), and cc me