[
https://issues.apache.org/jira/browse/DOXIA-590?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17540511#comment-17540511
]
ASF GitHub Bot commented on DOXIA-590:
--------------------------------------
feckertson commented on code in PR #98:
URL: https://github.com/apache/maven-doxia/pull/98#discussion_r878775059
##########
doxia-core/src/main/java/org/apache/maven/doxia/sink/impl/Xhtml5BaseSink.java:
##########
@@ -1567,21 +1567,24 @@ public void tableRow()
@Override
public void tableRow( SinkEventAttributes attributes )
{
- MutableAttributeSet att = new SinkEventAttributeSet();
+ MutableAttributeSet atts = SinkUtils.filterAttributes(
+ attributes, SinkUtils.SINK_TR_ATTRIBUTES );
- if ( evenTableRow )
+ if ( atts == null )
{
- att.addAttribute( Attribute.CLASS, "a" );
+ atts = new SinkEventAttributeSet();
}
- else
+
+ String rowClass = evenTableRow ? "a" : "b";
+ if ( atts.isDefined( Attribute.CLASS.toString() ) )
{
- att.addAttribute( Attribute.CLASS, "b" );
+ String givenRowClass = (String) atts.getAttribute(
Attribute.CLASS.toString() );
+ rowClass = givenRowClass + " " + rowClass;
Review Comment:
I am a fan of consistency and common code, but as far as I can tell the
other methods do not use more than one "default class" so tableRow is very
different from them. Also, I do not believe it is a problem if a single class
name is specified multiple times in an (X)HTML class string so the net effect
is the same for merge and always append when the thing being appended never
changes.
The only option which could produce a different result when the "default
class" never changes is "omit if a class string is provided", but that just
means "don't apply the default class style" which can be achieved with css.
The first additional tableRow method I discussed is not really that
different anyways. It will always apply "a" or "b", The consumer just gets to
control when it flips from one to the other.
The alternate additional tableRow method does feel a little different, but
it predicates on whether or not support for more than two stripe colors should
be supported out-of-the-box. Has anyone ever asked for such a thing? Besides,
it could also be achieved with css: override "a" and "b" to do nothing then
define and apply your own a', b', c'.
> Either provided element class or default class gets ignored
> -----------------------------------------------------------
>
> Key: DOXIA-590
> URL: https://issues.apache.org/jira/browse/DOXIA-590
> Project: Maven Doxia
> Issue Type: Bug
> Components: Core
> Affects Versions: 1.8
> Reporter: Fred Eckertson
> Assignee: Michael Osipov
> Priority: Major
> Fix For: 2.0.0-M3, 1.11.2
>
> Attachments: image-2022-05-18-21-57-40-619.png
>
>
> The following construct is somewhat common in doxia-core
> att.addAttribute( Attribute.CLASS, "a" );
> The documentation says that basic attributes (including CLASS) are supported.
> However in cases like this either that "a" or the CLASS that was provided in
> the attributes parameter will be ignored. The correct way to do this is to
> append the provided CLASS to "a " if a CLASS attribute was provided.
--
This message was sent by Atlassian Jira
(v8.20.7#820007)
