Closed Bug 54161 Opened 24 years ago Closed 24 years ago

"about:plugins" should be proper HTML

Categories

(SeaMonkey :: UI Design, defect, P2)

defect

Tracking

(Not tracked)

VERIFIED FIXED
Future

People

(Reporter: bugzilla, Assigned: alexeyc2003)

References

()

Details

Attachments

(7 files)

The "about:plugins" should be in correct HTML. Currently there are a lot of errors, like missing font end tags, no noscript tags, etc...
Keywords: patch, review
alas, i don't think this'll make it in for beta3...but nominating for rtm.
Keywords: rtm
diffs should happen against xpfe/global/resources/content/plugins.html. Looking..
I checked in some case changes, please re-do the diff on the file I specified, I tried applying your patch and got confused sorting through rejected hunks.
Status: NEW → ASSIGNED
Chris, what's bad if it's not proper HTML? Do we really need to take this for RTM?
Priority: P3 → P2
Whiteboard: [RTM NEED INFO]
When you redo it, add ``type="text/javascript"'' to the script tag. It's a required attribute.
Don: there is plenty of non-html 4.0 standard html floating around, this is not an rtm requirement. Jag: page loads ok w/o this, but Ok include it in the next diff.
OK, this is a minus!
Whiteboard: [RTM NEED INFO] → [rtm-]
Target Milestone: --- → Future
Removing patch keyword, need new patch.
Keywords: patch, review
Keywords: patch, review
The HTML file should go away anyway. See comments in bug 36908.
nav triage team: Marking nsbeta1-
Keywords: nsbeta1-
mcafee/alecf/jag, how does this patch look? okay or no, for checkin?
Keywords: rtm
This patch is really different, file has also moved. I fixed this in version 1.2 of mozilla/xpfe/global/resources/content/plugins.html with a patch from gemal, please reopen with a new patch if needed. marking fixed.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
I'm attaching a newer fix, which is outputting correct html and javascript. Gone is fx the font tag, etc. Please review and test.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
+if (numPlugins == 0) please use if (!numPlugins) otherwise it looks good.
Whiteboard: [rtm-]
Attached patch final diff...?:) (deleted) — Splinter Review
timeless: You're working with a count here and in such cases a comparison to |0| is clearer than a boolean "not" check. Compare |if (total == 0)| with |if (!total)|. So I suggest you use the version which compares numPlugins to 0. Henrik: I would personally put the |for| loop in the |else| part of that |if| statement. Yes, I know the loop won't do anything if numPlugins is 0, but putting it in the |else| more clearly indicates that logic, making the code easier to read and understand.
Please, feel free to optimize the patch...
I have 3 plugins installed, numPlugins reports 6, then I reload, it now reports 9, 12, 15, etc. I think the loop is ending on an error, when the index runs off data in the array. Any ideas? Debug by printing out numPlugins in the content.
Note, current version in CVS does not have JS error. Adding error for above comment: line 73, plugin has no properties.
*** Bug 61062 has been marked as a duplicate of this bug. ***
Keywords: mozilla0.8.1
Keywords: mozilla0.8
Mass-change: Do not remove nominations (even if Milestone passed). Readding mozilla0.8 nomination.
Keywords: mozilla0.8
Attached file plugins.html from above diff (deleted) —
In the attached diff I rewrote plugins.html in XHTML, which is currently the version of HTML recommended by W3C. Both, content and JS generated content are XHTML Strict and CSS2 valid, this is basically as proper as it gets: http://validator.w3.org/check?uri=http://bugzilla.mozilla.org/showattachment.cgi?attach_id=31988&doctype=Inline http://jigsaw.w3.org/css-validator/validator?uri=http://bugzilla.mozilla.org/showattachment.cgi?attach_id=31988 I have included JavaScript changes Mark Olson did in bug 61062 to make it correct JS as well. I have styled it to look exactly like the old plugins.html. However this can be changed to simplify the code or to improve presentation of the page. On a irrelevant note, because of the nature of our script, although the page is an XHTML document, it will work only with Mozilla's HTML parser, but not with XML parser. In order to parse it in XML the JS script must be extracted into a separate file. I have inserted the page into Mozilla and tested it in different conditions and it works like a charmer. In order to make the page XHTML compliant the document should start with a <!DOCTYPE, thus the disclaimer is not at the very top anymore. I'm not sure wether completely removing it will be a good idea, or wether the "<!-- -*- Mode: HTML;..." line will work with whatever editors it's supposed to work with. In same line I changed tab-width: from 8 to 2, to make the code a bit more compact. I didn't address the issue of internationalisation apart from specifying language of this document in <html> tag. I didn't include <noscript> tag because Mozilla executes JS inside about: pages regardles of wether JavaScript is turned on or off. The <?xml tag at the top is optional. CCing Ian Hickson and David Baron for their comments.
sr=alecf you know, about:plugins has looked the exact same since about Netscape 2.x. Does anyone want to beautify it? I think a nice xul outliner with some interactivity would be pretty nice.
If anyone wants to improve it, could they please allow users to disable plugins and mime handling? The attachment renders w/o table borders and w/o stretching in nc4.76, I hope that it works better in mozilla.
it looks fine in mozilla, with borders and everything.. just like 4.x (and 3.x, and 2.x, etc) in fact.
This attachment utilises CSS for presentation. NS4 does not fully support CSS. This page is supposed to be an internal part of Mozilla and is stored inside toolkit.jar Other browsers would not access it.
Yeah I know where it will live. The one interesting thing about most about: pages is that they generally work on random browsers. I can use netpositive 2 or 3's about page in the other version (or in Opera or...), i can take the mozilla or netscape about pages and use them in ie or something else. My comments are not a rejection of r= or anything; the fact that the functionality still works in netscape4 is a good thing. They're just musings.
Um, the document is invalid XML. You need to put the <script> section in a CDATA block (and remove the comment markers, which would make the entire <script> block dissappear and not be executed). Should we just go ahead and change the extension as well? Since it's XHTML, we might as well call it plugins.xml, no? Unfortunately, document.write and document.writeln are not available from within an XML document. Overall I would recommend simply making it an HTML4.01 page instead of an XML page, because of the above problems. If we go for XML, we could in fact just use XUL, of course... :-)
Ian, that is why I made a note that this will not work if parsed as XML. You're right, it's better off marking it as HTML 4.01 to avoid confusion/problems in the future. Attaching page modified as HTML 4.01 Strict. This one passes W3C Validator as well.
Attached file new plugins.html in HTML 4.01 Strict (deleted) —
Assuming we don't want to use XUL or XHTML, that we are happy with making the about:plugins file be hard to localise, that we don't mind abusing <dl>, and that we don't mind using awful APIs like document.write... r=hixie
It would be great if someone can do this in XUL. Until then, better to check this in, it fixes ocasional JavaScript errors and warnings with the current page.
fine with me, sr=alecf.. again.
build 20010509: JavaScript strict warning: jar:resource:///chrome/toolkit.jar!/content/global/plugins.html line 116: refere nce to undefined property plugin[j] more problems: - "Mime type" should be "MIME Type" - I'm getting no info about descriptions, suffixes, etc.. - URL to plugins should just be "http://netscape.com/plugins/"
fix was checked in yesterday, The descriptions are gone due XPCDOM landing the other day. You now will notice a lot of javascript errors happening all over the place from that. :o) For more info about this see bug 79743 I could not reproduce the JavaScript error you have on build 2001050915. Can you check if you get the same error with old version of plugins.html on latest buld? If you can, and didn't see it on older builds, this could be XPCDOM's doings as well. The URL is the same as in old plugins.html version. I think it's better for Netscape people to decide what it should be.
more problems #3 is real. Thanks for the corrected information. No, it's _not_ for netscape to choose, it's for mozilla.org to choose, and we have no interest in pointing to netcenter. and I hold my breath as I own blame for that line.
over to alex
Assignee: mcafee → alexeyc
Status: REOPENED → NEW
I see nothing wrong with pointing to netcenter if there is no better place for mozilla users to find plugins. I would only change it if you can find a place that better serves users of free software...
well, i shouldn't have added index.html to the url. nc4.77 has 'For more information on Netscape plug-ins, click here.' and the title of the page at the end of the link is Browser plug-ins. So we don't need to mention netscape netcenter.
about the strict javascript warning: --- Please add the following line to your prefs.js file, so we could avoid all the strict warning fixup...: user_pref("javascript.options.strict", true);
ah ok, yep I see it. That's the xpcdom bustage warning for bug 79743 The bug is fixed now, it is valid HTML and CSS. As far as I can see, the JS code is correct, and the URL is not broken. No need to worry about polish issues, because the work on further about:plugins development and improvement now mostly will continue in bug 56863, which will replace current plugins.html page. marking FIXED.
Status: NEW → RESOLVED
Closed: 24 years ago24 years ago
Resolution: --- → FIXED
bug 79743 has been fixed. The page works properly again, and there's no JS Strict warning. :o)
looks fine, and no js errors. vrfy fixed using 2001.05.29.0x comm bits on linux, winnt and mac.
Status: RESOLVED → VERIFIED
Product: Core → Mozilla Application Suite
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: