204727 - p3p stylesheets are buggy and inefficient

Status: RESOLVED WONTFIX

(Core :: XML, defect)

Description

[Axel Hecht]

p3p stylesheets contain things that shouldn't be there.
This needs some more investigation, but things that are apparent:

<xsl:if test = ".[@required='opt-in']"> wants a boolean, so this is illegal on
the one hand (abbreviated step with predicate) and useless, this should just be
<xsl:if test = "@required='opt-in'">

I saw at least one named template in one stylesheet ("ours") with just one call.
Those should be merged, IMHO.

Comment 1

[Axel Hecht]

taking, gonna do this in two steps.
First, I'm folding the 5 stylesheets into one template, using the preprocessor.pl
to generate the shipped stylesheets. That should make maintenance of the xslt
much simpler (or even feasable).
Then I'm going to optimize the stylesheet template itself (and it's callers).

Comment 2

[Axel Hecht]

Attachment: use preprocessor.pl to generate the stylesheets [Checked in: Comment 7]

This patch makes use of the preprocessor.pl to generate the stylesheets, as
they're mostly the same. I just change the defines that I call the preproc with

depending on what was in there.
I checked this on http://www.att.net/, and I compared the generated stylesheets

to their previous versions. (diff -uw, as preprocessor.pl changes the
newlines).
There are just whitespace differences.

Comment 3

[Axel Hecht]

Comment on attachment 133869 [details] [diff] [review]
use preprocessor.pl to generate the stylesheets
[Checked in: Comment 7]

requesting r/sr, bryner to make sure that I use preprocessor.pl right. Hixie
said he'd be the one to ask

Comment 4

[Brian Ryner (not reading)]

Comment on attachment 133869 [details] [diff] [review]
use preprocessor.pl to generate the stylesheets
[Checked in: Comment 7]

At one point, the preprocessor did not work with activestate perl.  Before we
enable this, we need to either make sure that it works, or update the Windows
build instructions for SeaMonkey to say that cygwin perl is required.

Comment 5

[Brian Ryner (not reading)]

Comment on attachment 133869 [details] [diff] [review]
use preprocessor.pl to generate the stylesheets
[Checked in: Comment 7]

Actually, bsmedberg tells me this does work on AS perl (see also bug 135533).

Comment 6

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Comment on attachment 133869 [details] [diff] [review]
use preprocessor.pl to generate the stylesheets
[Checked in: Comment 7]

Nice. I'd like to see you test this on at least one policy from each of the 5
versions that we support. Then add links to the sites (useful in future
testing).

sr=heikki

Comment 7

[Axel Hecht]

I tried three from the up-to-date compliant site page.
Sadly enough, the different files belong to different namespaces at some sites,
so we'd need some kind of compatibility matrix. I'll see what I can get.
Checked in attachement 133869.

Comment 8

[Axel Hecht]

This check-in caused quite some red. See bug 224557.
Bryner said to bsmedberg that requiring preprocessor.pl is fine, so the
tinderboxens will go thru an upgrade instead of backing out.
Posted to .builds, I hope somebody picks up the stick to update the build
instructions.

Comment 9

[Brendan Eich [:brendan]]

In comment 4, bryner said to update the build instructions before enabling this.
He used the royal "we", but that clearly is a circumlocution for "you, Axel" --
or whomever wants the patch to land.  Saying that "someone" should fix
tinderboxes or update build instructions is useless -- you might as well right
"no one".  Making work for others who didn't agree to that work is just bad
behavior.

What needs to happen here, or in bug 224557, so that there isn't any undone work
waiting for "someone"?

/be

Comment 10

[Brendan Eich [:brendan]]

Argh, tired -- "whomever" for "whoever" and "right" for "write".  I hope the
substance of that comment is valid, even if the form is sloppy.

/be

Comment 11

[Axel Hecht]

be, bryners comment was about activestate vs. cygwin perl and is, AFAICT, 
totally unrelated to the File::Spec issue in bug 224557. I myself used AS perl,
btw.

What needs to be done is actually not my decision, I would have updated the
build instructions a month ago, prolly, if we decided to leave the File::Spec
requirement in preprocessor.pl (bug 184182).

"we" in this case was prolly drivers. Oh, and there is just one tinderbox still 
affected, which may or may not die completely, given that there is nobody left
to maintain it. At least that's what Pav said (that there is nobody with access
to that machine). Note that I nailed the problem down in
http://bugzilla.mozilla.org/show_bug.cgi?id=224557#c17
(without access to the machine itself), there's just nothing happening.

Comment 12

[Benjamin Smedberg]

The build instructions at http://www.mozilla.org/build/unix-details.html#s3 are
already updated to require perl 5.6 or File::Spec 0.8.  What other instructions
need to be updated?

Comment 13

[[e-mail]]

p3p has been permanently cvs removed, marking wontfix.