www.mozilla.org/owners.html looks awful in Mozilla

RESOLVED FIXED

Status

RESOLVED FIXED
14 years ago
2 years ago

People

(Reporter: jerfa, Assigned: jwatt)

Tracking

Details

(URL)

Attachments

(2 attachments, 3 obsolete attachments)

(Reporter)

Description

14 years ago
User-Agent:       Mozilla/5.0
Build Identifier: 

Maybe related to bug 151557, the W3C validator reports 1033 errors for
http://www.mozilla.org/owners.html. Some of the tables are too wide, the worst
offender is the XML module. In Opera and IE the tables are all the same width,
this looks a whole lot better. The page doesn't even have a title!

Reproducible: Always
Steps to Reproduce:
This page is generated by despot...
Assignee: endico → leaf
Component: webmaster@mozilla.org → Despot
Product: mozilla.org → Webtools
QA Contact: whiteclover79-bug → timeless

Updated

14 years ago
Assignee: leaf → cmp
(Assignee)

Updated

14 years ago
Assignee: cmp → jonathan.watt
(Assignee)

Comment 2

14 years ago
Created attachment 175353 [details] [diff] [review]
patch to make things better

This should make things better - myk, can you mail me a copy of the generated
file so I can stick a <base> tag in it to pick up styling from mozilla.org?
Attachment #175353 - Flags: review?(myk)

Updated

14 years ago
Component: Despot → webmaster@mozilla.org
Product: Webtools → mozilla.org

Updated

14 years ago
Assignee: jonathan.watt → mozilla.webmaster
QA Contact: timeless → danielwang
(Assignee)

Updated

14 years ago
Assignee: mozilla.webmaster → jonathan.watt
Status: UNCONFIRMED → NEW
Component: webmaster@mozilla.org → Despot
Ever confirmed: true
Product: mozilla.org → Webtools
(Assignee)

Updated

14 years ago
Status: NEW → ASSIGNED
Comment on attachment 175353 [details] [diff] [review]
patch to make things better

>+                print OWNERS "    <a name='$name'></a>
>+    <table class="data">

You can't use double-quotes inside a double-quoted Perl string unless you
escape them.  But instead of escaping them, use the "qq|" quote operator, i.e.:

		print OWNERS qq|    <a name='$name'></a>
    <table class="data">
...
	  <a href='$maillist'>| . join(', ', @ownernames) . qq|</a>
...
	<td>|;

When you do this, change the single quoted HTML attributes to double-quoted
ones at the same time, i.e.:

		print OWNERS qq|    <a name="$name"></a>

Otherwise the code looks good, and the output looks a lot better than it used
to.  The only issue is that tables that don't need to be 100% wide aren't, and
they look funky interspersed with the rest of them.  All the tables should be
of equal width.
Attachment #175353 - Flags: review?(myk) → review-
(Assignee)

Comment 4

14 years ago
Created attachment 175419 [details] [diff] [review]
patch addressing comment 3

How does this look myk?
(Assignee)

Updated

14 years ago
Attachment #175353 - Attachment is obsolete: true
(Assignee)

Comment 5

14 years ago
One remaining issue I have is that the anchors aren't valid since they contain
spaces and other invalid characters. Is it okay to change them? Or will that
break too many links?

BTW I should have ended the patch with:

    </tbody>
    </table>
|;

Updated

14 years ago
Blocks: 151557
Comment on attachment 175419 [details] [diff] [review]
patch addressing comment 3

This patch doesn't apply cleanly to syncit.pl (neither the one in CVS nor the
one on despot.mozilla.org, which contains minor modifications).  Also, you need
to match closing to opening quote in this change:

+		 print OWNERS qq|</td>
       </tr>
+    </tbody>
     </table>

 ";
Attachment #175419 - Flags: review-
(Assignee)

Comment 7

14 years ago
Right. I mentioned that in my previous comment.

I don't understand why the patch doesn't apply for you. There have been no
checkins that conflict with my changes. What do I need to do to fix things?
I've checked in the minor modifications on despot.mozilla.org so they don't
interfere with this patch (and also because they should have been checked in a
long time ago).  You might want to cvs update your despot working copy and
update your patch.  AFAIK there's only one minor conflict between your patch and
the version now in CVS (line 148), although that's based on your first patch,
which I was able to apply by resolving that conflict.  I haven't figured out
what the conflicts are with the second patch.
(Assignee)

Comment 9

14 years ago
Created attachment 175493 [details] [diff] [review]
updated patch
Attachment #175419 - Attachment is obsolete: true
(Assignee)

Comment 10

14 years ago
Created attachment 175813 [details] [diff] [review]
patch now creating valid NAME tokens

This patch should also create valid NAME tokens from the partition names as
specified here:

  http://www.w3.org/TR/1999/REC-html401-19991224/types.html#type-cdata

It simply replaces consecutive occurances of invalid characters with a single
underscore. The output should still be sufficiently human readable. For
example:

  "Aurora/RDF BE"	       ->  "Aurora_RDF_BE"
  "BeOS-based gfx and widget"  ->  "BeOS-based_gfx_and_widget"
  "Find As You Type"	       ->  "Find_As_You_Type"
  "GFX and Widget - General"   ->  "GFX_and_Widget_-_General"
  "Image Handling: PNG"        ->  "Image_Handling:_PNG"
  "Xlib-based gfx + widget"    ->  "Xlib-based_gfx_widget"
(Assignee)

Updated

14 years ago
Attachment #175493 - Attachment is obsolete: true
Attachment #175813 - Flags: review?(myk)
Comment on attachment 175813 [details] [diff] [review]
patch now creating valid NAME tokens

Looks good, but Anne has a point about using hyphens rather than underscores. 
Please switch to hyphens before checking in.
Attachment #175813 - Flags: review?(myk) → review+
(Assignee)

Comment 13

14 years ago
Before I do that, can you send me the html this patch generates please? Also,
how does this seem:

sub name_to_id_token {
    ($_) = (@_);
    tr/A-Z/a-z/;
    s/[^A-Za-z0-9.]+/\-/g;
    return $_;
}
(In reply to comment #13)
> Before I do that, can you send me the html this patch generates please?

Sent.

> Also, how does this seem:
> 
> sub name_to_id_token {
>     ($_) = (@_);
>     tr/A-Z/a-z/;
>     s/[^A-Za-z0-9.]+/\-/g;
>     return $_;

Looks good.
Created attachment 177398 [details]
owners.html generated by despot with the latest patch applied
(Assignee)

Comment 16

14 years ago
Okay, I've checked in. Marking fixed. Thanks Myk!
Status: ASSIGNED → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED
Attachment #177398 - Attachment is patch: false
Attachment #177398 - Attachment mime type: text/plain → text/html
Product: Webtools → Webtools Graveyard
You need to log in before you can comment on or make changes to this bug.