You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@forrest.apache.org by Ferdinand Soethe <fe...@apache.org> on 2005/10/01 12:01:50 UTC

Cleaning up html-processing

After cleaning up a couple of rough edges in processing
and skinning table-elements in html-files I'm now stuck with the
problem of a border="1"-attribute appearing in table's that have a
class attribute in the source.

Searching for border, border="1" and table in all the way downwards
from webapp I found no more likely culprits, trying to follow the
pipelines I got stuck (see my previous post).

Can anybody tell me where that border="1" is coming from?

(I'm using a current head, did build clean and build and did a Forrest
clean before testing it)

Thanks
Ferdinand Soethe


Re: Cleaning up html-processing

Posted by Ferdinand Soethe <fe...@apache.org>.





Ferdinand Soethe wrote:






> Ferdinand Soethe wrote:


>> Sorry for the wild goose chase. What I still don't know is why this
>> doesn't work with my existing site (set up just a few weeks ago).
>> Since stuff like this has happened a few times I almost feel like I
>> should do all development on freshly seeded installations.

> Found at least one culprit. At some point in my dispair I had
> introduced an attribute border="none" in my table (which of course
> does not exist in html) and that apparently gets transformed to
> border="1" which ever way.

> Silly me ...

> Ferdinand Soethe


OK, David has also fixed that problems with ForrestTables. case
closed. Thanks guys ...


--
Ferdinand Soethe


Re: Cleaning up html-processing

Posted by Ferdinand Soethe <fe...@apache.org>.





Ferdinand Soethe wrote:


> Sorry for the wild goose chase. What I still don't know is why this
> doesn't work with my existing site (set up just a few weeks ago).
> Since stuff like this has happened a few times I almost feel like I
> should do all development on freshly seeded installations.

Found at least one culprit. At some point in my dispair I had
introduced an attribute border="none" in my table (which of course
does not exist in html) and that apparently gets transformed to
border="1" which ever way.

Silly me ...

Ferdinand Soethe


Re: General approach to backward compatibility

Posted by Ross Gardler <rg...@apache.org>.
David Crossley wrote:

> Issues should probably be described at our Jira.
> When closing it, declare the svn revision number
> that fixed it. Also in the svn commit, show the
> Jira issue number.

A little time saving hint:

There is no need to put the revision number in Jira as long as you put 
the JIRA number in the commit message (which you should do anyway). Jira 
will provide links directly to the viewsvn pages for that particular 
commit (including revision number).

Ross

Re: General approach to backward compatibility (was: Re: Cleaning up html-processing)

Posted by David Crossley <cr...@apache.org>.
Ferdinand Soethe wrote:
> 
> OK, since we all seem to agree on this I will write some piece for the
> updated docs next week and leave the changes.

Every major change should also be listed in
site-author/status.xml file. A brief note would
suffice and link to other info, such as the updating_*.html
and the Jira Issuue number.

That way people who use the current head SVN
are notified, and it builds the list of Changes.

Issues should probably be described at our Jira.
When closing it, declare the svn revision number
that fixed it. Also in the svn commit, show the
Jira issue number.

The tricky part is knowing when to do such effort
for the documentation.

-David

Re: General approach to backward compatibility (was: Re: Cleaning up html-processing)

Posted by Ferdinand Soethe <fe...@apache.org>.
OK, since we all seem to agree on this I will write some piece for the
updated docs next week and leave the changes.

--
Ferdinand Soethe


Re: General approach to backward compatibility (was: Re: Cleaning up html-processing)

Posted by Kevin <fo...@kegcl.demon.co.uk>.
On Fri, 2005-10-07 at 06:22 -0400, addi wrote:
> On Friday October 07 2005 3:06 am, Ferdinand Soethe wrote:
> > Ross Gardler wrote:
> > > One of our community has raised a concern,
> > > we have to address it. In this case I feel the call is yours as to
> > > whether the changes stay or not (others will speak up if they have an
> > > opinion).
> >
> > Yes 'others' please do!
> 
> Just piping up to say that I definitely agree that this is a fix that needs to 
> be addressed.  I think an explanation in the upgrade docs will suffice.

Yes adding a note to docs sounds good. My concern was a maybe and
I did say unlikely. 

Kevin

> - Addi
> 
> >
> > Problem is the interpretation of bugs and features here and I really
> > think community should clarify this once and for all.
> >
> > For better understanding:
> >
> > 1. The old behavior will overwrite any class-attributes in
> >    table-elements (of any type of document) with class="ForrestTable"
> >    and also set borders= and other presentational attributes right in the
> >    html-code.
> >
> >    My view is that this is broken functionality because
> >
> >    - it makes it impossible to custom format a table with a custom
> >      class-attribute and css (a documented feature).
> >
> >    - it goes against our concept of using css as much as possible.
> >
> >    In fact, a final solution should go even one step further and
> >    replace all presentational attributes in table with css-styling
> >    in the stylesheet.
> >
> >    In summary: I consider the changes to be fixes.  
> >
> > 2. Any change in existing behavior has the potential to break
> >    somebody's Forrest even if it is just fixing a bug.
> >
> >    For example: If somebody tried using their own class-attributes for
> >    table (which had no effect because it was swallowed) they will now
> >    find their custom class tables to have no borders and no more
> >    padding and will have to add extra-css to get them back.
> >
> >    (Pls. note that non-customized tables (without class-attributes) will
> >    continue to work as before!)
> >
> >    I figured that this would be considered to be an improvement
> >    because people who added class="xyz" in the first place likely did
> >    so to customize design, but who knows ...
> >
> > 3. While I accept the general policy of maintaining backward
> >    compatibility, I think this should be limited to documented features
> >    and should always consider what is involved in achieving that goal.
> >
> >    We should _not_ end up creating super complex stylesheets or
> >    bazillions of configuration options to ensure backward
> >    compatibility for bugs or leftovers from previous re-factoring
> >    steps (in this case moving from a non-css to a css-based design).
> >
> >    Especially if all that is required on upgrading would be a piece of
> >    css added to the extra-css in ones project. (A step which I agree
> >    should be documented in the upgrade info).
> >
> > wdyt?
> >
> >
> > --
> > Ferdinand Soethe


Re: General approach to backward compatibility (was: Re: Cleaning up html-processing)

Posted by addi <ad...@rocktreesky.com>.
On Friday October 07 2005 3:06 am, Ferdinand Soethe wrote:
> Ross Gardler wrote:
> > One of our community has raised a concern,
> > we have to address it. In this case I feel the call is yours as to
> > whether the changes stay or not (others will speak up if they have an
> > opinion).
>
> Yes 'others' please do!

Just piping up to say that I definitely agree that this is a fix that needs to 
be addressed.  I think an explanation in the upgrade docs will suffice.

- Addi

>
> Problem is the interpretation of bugs and features here and I really
> think community should clarify this once and for all.
>
> For better understanding:
>
> 1. The old behavior will overwrite any class-attributes in
>    table-elements (of any type of document) with class="ForrestTable"
>    and also set borders= and other presentational attributes right in the
>    html-code.
>
>    My view is that this is broken functionality because
>
>    - it makes it impossible to custom format a table with a custom
>      class-attribute and css (a documented feature).
>
>    - it goes against our concept of using css as much as possible.
>
>    In fact, a final solution should go even one step further and
>    replace all presentational attributes in table with css-styling
>    in the stylesheet.
>
>    In summary: I consider the changes to be fixes.  
>
> 2. Any change in existing behavior has the potential to break
>    somebody's Forrest even if it is just fixing a bug.
>
>    For example: If somebody tried using their own class-attributes for
>    table (which had no effect because it was swallowed) they will now
>    find their custom class tables to have no borders and no more
>    padding and will have to add extra-css to get them back.
>
>    (Pls. note that non-customized tables (without class-attributes) will
>    continue to work as before!)
>
>    I figured that this would be considered to be an improvement
>    because people who added class="xyz" in the first place likely did
>    so to customize design, but who knows ...
>
> 3. While I accept the general policy of maintaining backward
>    compatibility, I think this should be limited to documented features
>    and should always consider what is involved in achieving that goal.
>
>    We should _not_ end up creating super complex stylesheets or
>    bazillions of configuration options to ensure backward
>    compatibility for bugs or leftovers from previous re-factoring
>    steps (in this case moving from a non-css to a css-based design).
>
>    Especially if all that is required on upgrading would be a piece of
>    css added to the extra-css in ones project. (A step which I agree
>    should be documented in the upgrade info).
>
> wdyt?
>
>
> --
> Ferdinand Soethe

Re: General approach to backward compatibility

Posted by Ross Gardler <rg...@apache.org>.
My proposal for resolving such issues was snipped, so I'm pasting it 
again again here since a new thread has been started (I've generalised a 
little to fit the new subject line):

Ask yourself these two questions, then you'll know whether changes
are OK or not. I trust your judgment:

1) Does a site that rendered with 0.7 still look the same when
rendered with your modifications?

2) Does a site that failed to render correctly with 0.7 now render 
correctly?

If yes to both then changes are fine.

If no to the first then changes are not OK unless answer to second is
yes. In which case the changes will usually be OK (but they need
documenting in the upgrading to 0.8 documentation).

In the very last instance where it will usually be OK, it is only OK if
the community see it as OK. One of our community has raised a concern,
we have to address it. In this case I feel the call is yours as to
whether the changes stay or not (others will speak up if they have an
opinion).

----

In this specific instance, the answers appear to be:

1) Maybe (site now renders as documented, so this depends on user 
expectation)

2) Yes

With your additional explanation in this mail it seems pretty clear to 
me that, in this case, the changes are good ones. As long as we document 
the change then users will be aware of them and should not be stung by 
the answer to 1 (assuming they RTFM)

Ross

General approach to backward compatibility (was: Re: Cleaning up html-processing)

Posted by Ferdinand Soethe <fe...@apache.org>.
Ross Gardler wrote:

> One of our community has raised a concern, 
> we have to address it. In this case I feel the call is yours as to 
> whether the changes stay or not (others will speak up if they have an 
> opinion).

Yes 'others' please do!

Problem is the interpretation of bugs and features here and I really
think community should clarify this once and for all.

For better understanding:

1. The old behavior will overwrite any class-attributes in
   table-elements (of any type of document) with class="ForrestTable"
   and also set borders= and other presentational attributes right in the
   html-code.

   My view is that this is broken functionality because

   - it makes it impossible to custom format a table with a custom
     class-attribute and css (a documented feature).

   - it goes against our concept of using css as much as possible.

   In fact, a final solution should go even one step further and
   replace all presentational attributes in table with css-styling
   in the stylesheet.

   In summary: I consider the changes to be fixes.                                                      b

2. Any change in existing behavior has the potential to break
   somebody's Forrest even if it is just fixing a bug.

   For example: If somebody tried using their own class-attributes for
   table (which had no effect because it was swallowed) they will now
   find their custom class tables to have no borders and no more
   padding and will have to add extra-css to get them back.

   (Pls. note that non-customized tables (without class-attributes) will
   continue to work as before!)
   
   I figured that this would be considered to be an improvement
   because people who added class="xyz" in the first place likely did
   so to customize design, but who knows ...

3. While I accept the general policy of maintaining backward
   compatibility, I think this should be limited to documented features
   and should always consider what is involved in achieving that goal.

   We should _not_ end up creating super complex stylesheets or
   bazillions of configuration options to ensure backward
   compatibility for bugs or leftovers from previous re-factoring
   steps (in this case moving from a non-css to a css-based design).

   Especially if all that is required on upgrading would be a piece of
   css added to the extra-css in ones project. (A step which I agree
   should be documented in the upgrade info).

wdyt?
   

--
Ferdinand Soethe


Re: Cleaning up html-processing

Posted by Ross Gardler <rg...@apache.org>.
Ferdinand Soethe wrote:
> Ross Gardler wrote:
> 
> 
>>Any solution applied to SVN should not change existing behaviour, even
>>if that behaviour does not suit you.
> 
> 
>>Make it configurable or, if you are using 0.8-dev you can simply add a
>>project locationmap with a match for the stylesheet you don't like and
>>provide your own modified one in your project. No need to create a whole
>>new skin.
> 
> 
> Hmmm. This comes as a bit of a shock as it basically means that all
> the changes to the skinning that I did during the last few month would
> have to made configurable (Nobody mentioned this when I discussed them
> on-list).

Then, if these changes have altered the behaviour for o.7 then this may 
be oversight of the community as a whole. Read on...

> In a case like this table-element (or my earlier fixes with ID)
> this would require switches in a number of different places, in my view a
> nightmare to write, document and maintain.

So, would the locaitonmap route be the right way here, unless...

> I understand that the above rules apply to changing existing
> _features_, but should we really apply them to bugfixes and changes
> that make Forrest behave as documented?

I'm confused by all this, on the one hand someone is saying that it 
breaks backward compatability, on the other hand, someone else is saying 
it never worked before anyway.  My comments so are only in response to 
the concern that it breaks backward compatability, if that is not the 
case then you can ignore my comments.

Ask yourself these two questions, then you'll know whether these changes 
are OK or not. I trust your judgment:

1) Does an HTML page that rendered with 0.7 still look the same when 
rendered with your modified stylesheets?

2) Does an HTML page that failed to render with 0.7 now render?

If yes to both then changes are fine.

If no to the first then changes are not OK unless answer to second is 
yes. In which case the changes will usually be OK (but they need 
documenting in the upgrading to 0.8 documentation).

In the very last instance where it will usually be OK, it is only OK if 
the community see it as OK. One of our community has raised a concern, 
we have to address it. In this case I feel the call is yours as to 
whether the changes stay or not (others will speak up if they have an 
opinion).

Ross

Re: Cleaning up html-processing

Posted by Ferdinand Soethe <fe...@apache.org>.
Ross Gardler wrote:

> Any solution applied to SVN should not change existing behaviour, even
> if that behaviour does not suit you.

> Make it configurable or, if you are using 0.8-dev you can simply add a
> project locationmap with a match for the stylesheet you don't like and
> provide your own modified one in your project. No need to create a whole
> new skin.

Hmmm. This comes as a bit of a shock as it basically means that all
the changes to the skinning that I did during the last few month would
have to made configurable (Nobody mentioned this when I discussed them
on-list).

In a case like this table-element (or my earlier fixes with ID)
this would require switches in a number of different places, in my view a
nightmare to write, document and maintain.

I understand that the above rules apply to changing existing
_features_, but should we really apply them to bugfixes and changes
that make Forrest behave as documented?

For example: The state of the table-element processing that Kevin
observed as 'working' was only after I had applied some first repairs
that stopped Forrest overwriting _any_ class-attribute with
'ForrestTable'. Since in our docs we suggest customizing Forrest by
inserting custom class-attributes and styling them with extra-css, I
considered this a bug to be fixed. And still do.

So pls let me know what you think. The changes to the 0.7 fix branch can
easily be rolled back if need be. To roll back the changes to 0.8 I'd
probably need some help as they go back some month.


--
Ferdinand Soethe


Re: Cleaning up html-processing

Posted by Ross Gardler <rg...@apache.org>.
Ferdinand Soethe wrote:
> Kevin wrote:
> 
> 
>>Thanks for sticking with me. Just one point to finish with.
> 
> 
>>The new code bypasses old working code that some web sites
>>may depend on? A user may have been happy with the default
>>cellpadding and cellspacing while setting their own class.
> 
> 
>>Just a backward compatibility issue, I'm sure unlikely.
> 
> 
> Yeah, I thought of that too. But then, fixing what I consider
> inconsistencies and bugs in the old skin I always take the chance that
> sbdy depends on just this behavior.
> 
> What do others think? Should I roll back the changes and derive a new
> skin to apply these and other 'fixes'?

Any solution applied to SVN should not change existing behaviour, even 
if that behaviour does not suit you.

Make it configurable or, if you are using 0.8-dev you can simply add a 
project locationmap with a match for the stylesheet you don't like and 
provide your own modified one in your project. No need to create a whole 
new skin.

Ross

Re: Cleaning up html-processing

Posted by Ferdinand Soethe <fe...@apache.org>.
Kevin wrote:

> Thanks for sticking with me. Just one point to finish with.

> The new code bypasses old working code that some web sites
> may depend on? A user may have been happy with the default
> cellpadding and cellspacing while setting their own class.

> Just a backward compatibility issue, I'm sure unlikely.

Yeah, I thought of that too. But then, fixing what I consider
inconsistencies and bugs in the old skin I always take the chance that
sbdy depends on just this behavior.

What do others think? Should I roll back the changes and derive a new
skin to apply these and other 'fixes'?

--
Ferdinand Soethe


Re: Cleaning up html-processing

Posted by Kevin <fo...@kegcl.demon.co.uk>.
Hi Ferdinand,

Thanks for sticking with me. Just one point to finish with.

The new code bypasses old working code that some web sites
may depend on? A user may have been happy with the default
cellpadding and cellspacing while setting their own class.

Just a backward compatibility issue, I'm sure unlikely.

Kevin


On Thu, 2005-10-06 at 08:42 +0200, Ferdinand Soethe wrote:
> Hi Kevin,
> 
> thanks for taking the time to explain.
> Re-reading a forth time I now understand that with
> 
> > I think should be:
> > 
> >  <xsl:when test="not(@class)">
> 
> you actually suggested part of the fix that David later committed.
> I'm sorry. I just saw 'not' and your explanations below and concluded
> that you wanted to turn around my conditions. Should have looked at
> the code a little closer.
> 
> > is not needed as the original version worked fine. I wasn't
> > suggesting any new approach.
> 
> Well in that point I still disagree. In order for the old version
> below to work ok you have to overwrite _all_ the attributes that are set
> or you'll end up with some attributes remaining unchanged.
> 
> > <xsl:template match="table">
> >     <xsl:apply-templates select="@id"/>
> 
> >     <table cellpadding="4" cellspacing="1" class="ForrestTable">
> >           <xsl:copy-of select="@cellspacing | @cellpadding |
> > @border | @class | @bgcolor |@id"/>
> >           <xsl:apply-templates/>
> >     </table>
> 
> My goal however was to provide a clean alternative for css-formatting
> and that meant I wanted _no_ spacing and padding attributes in the
> html-code.
> 
> So I still believe that the new solution is cleaner and better than
> the old code.
> 
> > so now what worked is partly redundant. Sorry I can't explain,
> > I'm doubting myself now.
> 
> If I once again missed something, let me know.
> 
> --
> Ferdinand Soethe
> 


Re: Cleaning up html-processing

Posted by Ferdinand Soethe <fe...@apache.org>.
Hi Kevin,

thanks for taking the time to explain.
Re-reading a forth time I now understand that with

> I think should be:
> 
>  <xsl:when test="not(@class)">

you actually suggested part of the fix that David later committed.
I'm sorry. I just saw 'not' and your explanations below and concluded
that you wanted to turn around my conditions. Should have looked at
the code a little closer.

> is not needed as the original version worked fine. I wasn't
> suggesting any new approach.

Well in that point I still disagree. In order for the old version
below to work ok you have to overwrite _all_ the attributes that are set
or you'll end up with some attributes remaining unchanged.

> <xsl:template match="table">
>     <xsl:apply-templates select="@id"/>

>     <table cellpadding="4" cellspacing="1" class="ForrestTable">
>           <xsl:copy-of select="@cellspacing | @cellpadding |
> @border | @class | @bgcolor |@id"/>
>           <xsl:apply-templates/>
>     </table>

My goal however was to provide a clean alternative for css-formatting
and that meant I wanted _no_ spacing and padding attributes in the
html-code.

So I still believe that the new solution is cleaner and better than
the old code.

> so now what worked is partly redundant. Sorry I can't explain,
> I'm doubting myself now.

If I once again missed something, let me know.

--
Ferdinand Soethe


Re: Cleaning up html-processing

Posted by Kevin <fo...@kegcl.demon.co.uk>.
On Wed, 2005-10-05 at 23:27 +0200, Ferdinand Soethe wrote:
> Kevin wrote:
> 
> > This is not my approach. It is in document2html.xsl code anyway.
> 
> > Look at line after new <xsl:when test="not(@class) or @class=''">
> 
> After re-reading the whole thread a third time I still don't get what
> your approach is. Would you mind explaining it once more?

I was saying that adding:

<xsl:when test="not(@class) or @class=''">

is not needed as the original version worked fine. I wasn't
suggesting any new approach.

<xsl:template match="table">
    <xsl:apply-templates select="@id"/>

    <table cellpadding="4" cellspacing="1" class="ForrestTable">
          <xsl:copy-of select="@cellspacing | @cellpadding | @border | @class | @bgcolor |@id"/>
          <xsl:apply-templates/>
    </table>

exists and did the job. You have wrapped something that worked
with

<xsl:when test="not(@class) or @class=''">

<!-- above working code deals with Tables with class -->

</xsl:when>
<xsl:otherwise>
        <!-- Tables with class are passed without change -->
        <xsl:copy>
          <xsl:copy-of select="@*"/>
          <xsl:apply-templates/>
        </xsl:copy>    
</xsl:otherwise>

so now what worked is partly redundant. Sorry I can't explain,
I'm doubting myself now.

Kevin


> Thanks,
> Ferdinand Soethe
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 


Re: Cleaning up html-processing

Posted by Ferdinand Soethe <fe...@apache.org>.





Kevin wrote:

> This is not my approach. It is in document2html.xsl code anyway.

> Look at line after new <xsl:when test="not(@class) or @class=''">

After re-reading the whole thread a third time I still don't get what
your approach is. Would you mind explaining it once more?

Thanks,
Ferdinand Soethe













Re: Cleaning up html-processing

Posted by Kevin <fo...@kegcl.demon.co.uk>.
On Wed, 2005-10-05 at 08:26 +0200, Ferdinand Soethe wrote:
> Ross Gardler wrote:
> 
> > Sorry, I didn't follow this thread closely, I just noticed David had 
> > corrected Ferdinands commit. You seem to be suggesting that The commit
> > should never have been made. Ferdinand, is that the case now that you 
> > have found some other issues with what you were doing?
> 
> No, as I wrote yesterday, David's fix solved all the problem I had/saw
> with this. The approach Kevin suggested is a feasible but different
> solution that first writes the Forrest default settings

> > <!-- default cellpadding and cellspacing and class -->
> >     <table cellpadding="4" cellspacing="1" class="ForrestTable">
> 
> and then overwrites them with those attributes present in the source
> 
> > <!-- if cellpadding or cellspacing or class overwrite defaults  -->
> >       <xsl:copy-of select="@cellspacing | @cellpadding | @border |
> > @class | @bgcolor"/>

This is not my approach. It is in document2html.xsl code anyway.

Look at line after new <xsl:when test="not(@class) or @class=''">

:)

Kevin


> the outcome being that you can create a mix of Forrest's default
> attributes and attributes that you set in the source (for both Forrest
> and other table-elements).
> 
> --
> Ferdinand Soethe
> 


Re: Cleaning up html-processing

Posted by Ferdinand Soethe <fe...@apache.org>.
Ross Gardler wrote:

> Sorry, I didn't follow this thread closely, I just noticed David had 
> corrected Ferdinands commit. You seem to be suggesting that The commit
> should never have been made. Ferdinand, is that the case now that you 
> have found some other issues with what you were doing?

No, as I wrote yesterday, David's fix solved all the problem I had/saw
with this. The approach Kevin suggested is a feasible but different
solution that first writes the Forrest default settings

> <!-- default cellpadding and cellspacing and class -->
>     <table cellpadding="4" cellspacing="1" class="ForrestTable">

and then overwrites them with those attributes present in the source

> <!-- if cellpadding or cellspacing or class overwrite defaults  -->
>       <xsl:copy-of select="@cellspacing | @cellpadding | @border |
> @class | @bgcolor"/>

the outcome being that you can create a mix of Forrest's default
attributes and attributes that you set in the source (for both Forrest
and other table-elements).

--
Ferdinand Soethe


Re: Cleaning up html-processing

Posted by Ross Gardler <rg...@apache.org>.
Kevin wrote:
> On Tue, 2005-10-04 at 17:24 +0100, Ross Gardler wrote:
> 
>>Ferdinand Soethe wrote:
>>
>>
>>>Even though class is empty and
>>>
>>>"<xsl:value-of select="@class"/>" will show ""
>>>
>>>the test condition
>>>
>>><xsl:when test="@class = ''">
>>>
>>>does not seem to work properly so all tables are now
>>>treated like in the otherwise branch.
>>
>>See Davids commit: 
>>http://svn.apache.org/viewcvs.cgi/forrest/trunk/main/webapp/skins/common/xslt/html/document2html.xsl?rev=293531&r1=292938&r2=293531&diff_format=h
>>
>>Ross
> 
> 
> I was suggesting undoing the change and going back to r232890,
> thought it did the job correctly? Am I missing something? I've
> added comments to template below.

Sorry, I didn't follow this thread closely, I just noticed David had 
corrected Ferdinands commit. You seem to be suggesting that The commit 
should never have been made. Ferdinand, is that the case now that you 
have found some other issues with what you were doing?

(I left Kevins comments below)

Ross

> 
> document2html.xsl:
> 
>   <xsl:template match="table">
>     <xsl:apply-templates select="@id"/>
> 
> <!-- default cellpadding and cellspacing and class -->
>     <table cellpadding="4" cellspacing="1" class="ForrestTable">
> 
> <!-- if cellpadding or cellspacing or class overwrite defaults  -->
>       <xsl:copy-of select="@cellspacing | @cellpadding | @border |
> @class | @bgcolor"/>
> 
>       <xsl:apply-templates/>
>     </table>
>   </xsl:template
> 
> Kevin
> 
> 
> 
> 
> 
> 


Re: Cleaning up html-processing

Posted by Ferdinand Soethe <fe...@apache.org>.





Kevin wrote:

> I was suggesting undoing the change and going back to r232890,
> thought it did the job correctly? Am I missing something? I've
> added comments to template below.

Sorry for not responding to this. While I was writing a response David
fixed to problem.

> Am I missing something?

Yes, you were. The choose switch as such was correct as it is meant to
treat table-elements that have no "class" (in the source) as standard
Forrest tables and add class="ForrestTable" etc. while table-elements
that already have a class should not be changes so that customization
based on extra-css and class can take place.

My mistake was the evaluation of the class-attribute in the switch
which becomes obvious when you look at Davids fix

-      <!-- Limit Forrest specific processing  to tables without class -->
-      <xsl:when test="@class = ''">
+      <!-- Limit Forrest specific processing to tables without class -->
+      <xsl:when test="not(@class) or @class=''">
         <table cellpadding="4" cellspacing="1" class="ForrestTable">
           <xsl:copy-of select="@cellspacing | @cellpadding | @border | @class | @bgcolor |@id"/>
           <xsl:apply-templates/>

My test="@class=''" would not work if class was not present at all so
David changed that test="not(@class) or @class=''".

Thanks for your help,
Ferdinand





Re: Cleaning up html-processing

Posted by Kevin <fo...@kegcl.demon.co.uk>.
On Tue, 2005-10-04 at 17:24 +0100, Ross Gardler wrote:
> Ferdinand Soethe wrote:
> 
> > Even though class is empty and
> > 
> > "<xsl:value-of select="@class"/>" will show ""
> > 
> > the test condition
> > 
> > <xsl:when test="@class = ''">
> > 
> > does not seem to work properly so all tables are now
> > treated like in the otherwise branch.
> 
> See Davids commit: 
> http://svn.apache.org/viewcvs.cgi/forrest/trunk/main/webapp/skins/common/xslt/html/document2html.xsl?rev=293531&r1=292938&r2=293531&diff_format=h
> 
> Ross

I was suggesting undoing the change and going back to r232890,
thought it did the job correctly? Am I missing something? I've
added comments to template below.

document2html.xsl:

  <xsl:template match="table">
    <xsl:apply-templates select="@id"/>

<!-- default cellpadding and cellspacing and class -->
    <table cellpadding="4" cellspacing="1" class="ForrestTable">

<!-- if cellpadding or cellspacing or class overwrite defaults  -->
      <xsl:copy-of select="@cellspacing | @cellpadding | @border |
@class | @bgcolor"/>

      <xsl:apply-templates/>
    </table>
  </xsl:template

Kevin





Re: Cleaning up html-processing

Posted by Ross Gardler <rg...@apache.org>.
Ferdinand Soethe wrote:

> Even though class is empty and
> 
> "<xsl:value-of select="@class"/>" will show ""
> 
> the test condition
> 
> <xsl:when test="@class = ''">
> 
> does not seem to work properly so all tables are now
> treated like in the otherwise branch.

See Davids commit: 
http://svn.apache.org/viewcvs.cgi/forrest/trunk/main/webapp/skins/common/xslt/html/document2html.xsl?rev=293531&r1=292938&r2=293531&diff_format=h

Ross

Re: Cleaning up html-processing

Posted by Ferdinand Soethe <fe...@apache.org>.

Ross Gardler wrote:

> I also was unable to find a border="1" sing search tools. Perhaps we can
> work with Ferdinand to solve this on todays Forest Tuesday.

Thanks. After Kevin's posting I got suspicious and tried a freshly
seed site this morning. Border = 1 no longer shows and class is
properly passed through.

Sorry for the wild goose chase. What I still don't know is why this
doesn't work with my existing site (set up just a few weeks ago).
Since stuff like this has happened a few times I almost feel like I
should do all development on freshly seeded installations.

But while we are at it Kevin has uncovered a programming error for
tables without class that I created but don't understand:

> <html>
> <head><title>Tables</title></head>
> <body>
> <h1>Borders?</h1>
> <table><tr><td>Forrest Table</td></tr></table>
> <table class="foo" cellpadding="5"><tr><td>Foo Table</td></tr></table>
> </body>
> </html>
> 
> Generated Forrest page had no borders on either table
> and no expected ForrestTable class on <table> with no class.


Even though class is empty and

"<xsl:value-of select="@class"/>" will show ""

the test condition

<xsl:when test="@class = ''">

does not seem to work properly so all tables are now
treated like in the otherwise branch.

--
Ferdinand Soethe


Re: Cleaning up html-processing

Posted by Ferdinand Soethe <fe...@apache.org>.





Ross Gardler wrote:

> I also was unable to find a border="1" sing search tools. Perhaps we can
> work with Ferdinand to solve this on todays Forest Tuesday.

Thanks. Am on the road right now but will try to join in later today.


--
Ferdinand Soethe


Re: Cleaning up html-processing

Posted by Ross Gardler <rg...@apache.org>.
David Crossley wrote:
> Kevin wrote:
> 
>>Ferdinand Soethe wrote: 
>>
>>
>>>But using these new transformation it becomes obvious that it is no
>>>longer a css-Effekt. If you make Forrest transform an html-file
>>>with something like
>>>
>>><table class="foo">
>>>.....
>>>
>>>The resulting Forrest page will have
>>>
>>><table class="foo"  border="1">
>>>....
>>
>>I can't reproduce the border="1" problem. I tried a basic test.
>>
>><html>
>><head><title>Tables</title></head>
>><body>
>><h1>Borders?</h1>
>><table><tr><td>Forrest Table</td></tr></table>
>><table class="foo" cellpadding="5"><tr><td>Foo Table</td></tr></table>
>></body>
>></html>
>>
>>Generated Forrest page had no borders on either table
>>and no expected ForrestTable class on <table> with no class.
>>
>>Going back to document2html.xsl before update:
>>
>>  <xsl:template match="table">
>>    <xsl:apply-templates select="@id"/>
>>    <table cellpadding="4" cellspacing="1" class="ForrestTable">
>>      <xsl:copy-of select="@cellspacing | @cellpadding | @border |
>>@class | @bgcolor"/>
>>      <xsl:apply-templates/>
>>    </table>
>>  </xsl:template>
>>
>>If @class and other attributes are set then don't they copy over
>>class="ForrestTable" cellpadding="4" cellspacing="1" so only getting
>>defaults when not set. So works as expected.
>>
>>Sorry can't help on the border="1" it must be comming into this xsl.
>>
>>Kevin
>>
>>
>>>and my problem is that I have not found out where this is coming from.
>>>
>>>Any ideas?
> 
> 
> I had to contruct a hefty find|grep|grep to exclude other mentions
> of border stuff, so might have excluded some other relevant things.
> 
> Other than some CSS which sets border=1px, i cannot find anything
> other than one mention in skins/tigris/xslt/html/document2html.xsl
> and a commented-out section in main/webapp/resources/stylesheets/page2html.xsl

I also was unable to find a border="1" sing search tools. Perhaps we can 
work with Ferdinand to solve this on todays Forest Tuesday.

Ross

Re: Cleaning up html-processing

Posted by David Crossley <cr...@apache.org>.
Kevin wrote:
> Ferdinand Soethe wrote: 
> 
> > But using these new transformation it becomes obvious that it is no
> > longer a css-Effekt. If you make Forrest transform an html-file
> > with something like
> > 
> > <table class="foo">
> > .....
> > 
> > The resulting Forrest page will have
> > 
> > <table class="foo"  border="1">
> > ....
> 
> I can't reproduce the border="1" problem. I tried a basic test.
> 
> <html>
> <head><title>Tables</title></head>
> <body>
> <h1>Borders?</h1>
> <table><tr><td>Forrest Table</td></tr></table>
> <table class="foo" cellpadding="5"><tr><td>Foo Table</td></tr></table>
> </body>
> </html>
> 
> Generated Forrest page had no borders on either table
> and no expected ForrestTable class on <table> with no class.
> 
> Going back to document2html.xsl before update:
> 
>   <xsl:template match="table">
>     <xsl:apply-templates select="@id"/>
>     <table cellpadding="4" cellspacing="1" class="ForrestTable">
>       <xsl:copy-of select="@cellspacing | @cellpadding | @border |
> @class | @bgcolor"/>
>       <xsl:apply-templates/>
>     </table>
>   </xsl:template>
> 
> If @class and other attributes are set then don't they copy over
> class="ForrestTable" cellpadding="4" cellspacing="1" so only getting
> defaults when not set. So works as expected.
> 
> Sorry can't help on the border="1" it must be comming into this xsl.
> 
> Kevin
> 
> > and my problem is that I have not found out where this is coming from.
> > 
> > Any ideas?

I had to contruct a hefty find|grep|grep to exclude other mentions
of border stuff, so might have excluded some other relevant things.

Other than some CSS which sets border=1px, i cannot find anything
other than one mention in skins/tigris/xslt/html/document2html.xsl
and a commented-out section in main/webapp/resources/stylesheets/page2html.xsl

-David


Re: Cleaning up html-processing

Posted by Kevin <fo...@kegcl.demon.co.uk>.
On Sun, 2005-10-02 at 22:20 +0200, Ferdinand Soethe wrote: 
> Thanks Kevin,
> 
> > Don't think there is. The border effect is achieved by setting the
> > background colour in profile.css and using a cellspacing="1" on all
> > ForrestTable classes.
> 
> and yes. That may be true for ForrestTable classes. But the source I'm
> talking about has a different class attribute and the changes I've
> committed yesterday make sure that this different class is no longer
> overwritten with ForrestTable in the process.

Is this the change to document2html.xsl?

<!-- Limit Forrest specific processing  to tables without class -->
      <xsl:when test="@class = ''">
      <table cellpadding="4" cellspacing="1" class="ForrestTable">
      ...

I think should be:

 <xsl:when test="not(@class)">

See below about missing class="ForrestTable" when class is not set.

> But using these new transformation it becomes obvious that it is no
> longer a css-Effekt. If you make Forrest transform an html-file
> with something like
> 
> <table class="foo">
> .....
> 
> The resulting Forrest page will have
> 
> <table class="foo"  border="1">
> ....

I can't reproduce the border="1" problem. I tried a basic test.

<html>
<head><title>Tables</title></head>
<body>
<h1>Borders?</h1>
<table><tr><td>Forrest Table</td></tr></table>
<table class="foo" cellpadding="5"><tr><td>Foo Table</td></tr></table>
</body>
</html>

Generated Forrest page had no borders on either table
and no expected ForrestTable class on <table> with no class.

Going back to document2html.xsl before update:

  <xsl:template match="table">
    <xsl:apply-templates select="@id"/>
    <table cellpadding="4" cellspacing="1" class="ForrestTable">
      <xsl:copy-of select="@cellspacing | @cellpadding | @border |
@class | @bgcolor"/>
      <xsl:apply-templates/>
    </table>
  </xsl:template>

If @class and other attributes are set then don't they copy over
class="ForrestTable" cellpadding="4" cellspacing="1" so only getting
defaults when not set. So works as expected.

Sorry can't help on the border="1" it must be comming into this xsl.

Kevin

> 
> and my problem is that I have not found out where this is coming from.
> 
> Any ideas?
> 
> --
> Ferdinand Soethe
> 


Re: Cleaning up html-processing

Posted by Ferdinand Soethe <fe...@apache.org>.
Thanks Kevin,

> Don't think there is. The border effect is achieved by setting the
> background colour in profile.css and using a cellspacing="1" on all
> ForrestTable classes.

and yes. That may be true for ForrestTable classes. But the source I'm
talking about has a different class attribute and the changes I've
committed yesterday make sure that this different class is no longer
overwritten with ForrestTable in the process.

But using these new transformation it becomes obvious that it is no
longer a css-Effekt. If you make Forrest transform an html-file
with something like

<table class="foo">
.....

The resulting Forrest page will have

<table class="foo"  border="1">
....

and my problem is that I have not found out where this is coming from.

Any ideas?

--
Ferdinand Soethe


Re: Cleaning up html-processing

Posted by Kevin <fo...@kegcl.demon.co.uk>.
On Sat, 2005-10-01 at 12:01 +0200, Ferdinand Soethe wrote:
> After cleaning up a couple of rough edges in processing
> and skinning table-elements in html-files I'm now stuck with the
> problem of a border="1"-attribute appearing in table's that have a
> class attribute in the source.
> 
> Searching for border, border="1" and table in all the way downwards
> from webapp I found no more likely culprits, trying to follow the
> pipelines I got stuck (see my previous post).
> 
> Can anybody tell me where that border="1" is coming from?

Don't think there is. The border effect is achieved by setting the
background colour in profile.css and using a cellspacing="1" on all
ForrestTable classes.

profile.css.xslt:

<xsl:template match="color[@name='table']">
.ForrestTable      { background-color: <xsl:value-of select="@value"/>;}
</xsl:template>

generates

profile.css:

.ForrestTable      { background-color: #ccc;}

(do a forrest site and play with this color)

document2html.xsl:

  <xsl:template match="table">
    <xsl:apply-templates select="@id"/>
    <table cellpadding="4" cellspacing="1" class="ForrestTable">
      <xsl:copy-of select="@cellspacing | @cellpadding | @border |
@class | @bgcolor |@id"/>
      <xsl:apply-templates/>
    </table>
  </xsl:template>

generates output html. I may not have the full story but it's
along those lines.

Kevin

> (I'm using a current head, did build clean and build and did a Forrest
> clean before testing it)
> 
> Thanks
> Ferdinand Soethe
>