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

Status: RESOLVED FIXED

(Webtools Graveyard :: Despot, defect)

Description

[Erik Fabert]

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:

Comment 1

[Dave Miller [:justdave]]

This page is generated by despot...

Comment 2

[Jonathan Watt [:jwatt]]

Attachment: 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?

Comment 3

[Myk Melez [:myk] [@mykmelez]]

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.

Comment 4

[Jonathan Watt [:jwatt]]

Attachment: patch addressing comment 3

How does this look myk?

Comment 5

[Jonathan Watt [:jwatt]]

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>
|;

Comment 6

[Myk Melez [:myk] [@mykmelez]]

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>

 ";

Comment 7

[Jonathan Watt [:jwatt]]

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?

Comment 8

[Myk Melez [:myk] [@mykmelez]]

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.

Comment 9

[Jonathan Watt [:jwatt]]

Attachment: updated patch

Comment 10

[Jonathan Watt [:jwatt]]

Attachment: 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"

Comment 11

[Anne (:annevk)]

I think we want them to be lowercase and hyphenated per:
 <http://www.mozilla.org/contribute/writing/guidelines#linking>
 <http://www.mozilla.org/contribute/writing/guidelines#filenames>

Comment 12

[Myk Melez [:myk] [@mykmelez]]

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.

Comment 13

[Jonathan Watt [:jwatt]]

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 $_;
}

Comment 14

[Myk Melez [:myk] [@mykmelez]]

(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.

Comment 15

[Myk Melez [:myk] [@mykmelez]]

Attachment: owners.html generated by despot with the latest patch applied

Comment 16

[Jonathan Watt [:jwatt]]

Okay, I've checked in. Marking fixed. Thanks Myk!