Hi Krishna,
Thank you for dropping </li> tags.
*DesktopProperties.html:*
61 <tr bgcolor="#ccccff">
bgcolor attribute is obsolete since HTML5 [1][2]. I propose changing it to
style="background-color: #ccccff"
It gives the same result and is standards-compliant.
*Modality.html*
I also propose removing, or rather not adding <style> element.
Adding style="text-align: center" to <tbody> element gives the same
result because ‘text-align’ property is inherited. Then you won't need
to override text-align in the second table.
You removed too many </p>'s.
There should be </p> at line 209 which closes <p> opened at line 206.
Here <ol> is misaligned:
441 <td style="text-align:left">
442 <ol>
But it seems to be in the correct column in the previous version. Are
there any tabs involved? The line isn't marked as having any changes.
Regards,
Alexey
P.S. Ah, I completely missed the fact the second table contains images
in its second column.
[1] https://www.w3.org/TR/html52/obsolete.html#element-attrdef-tr-bgcolor
[2]
https://developer.mozilla.org/en-US/docs/Web/HTML/Element/tr#attr-bgcolor
On 28/11/2018 11:41, Krishna Addepalli wrote:
Hi Alexey, Sergey,
Thanks for your suggestions. I have removed the </li> tag and also removed the
style header from DesktopProperties.html.
I have removed the commented out lines and empty tags as well from
Modality.html, but have not changed anything else as suggested by Alexey.
Perhaps, addressing them in separate bug is better?
Here is the updated webrev:
http://cr.openjdk.java.net/~kaddepalli/8213048/webrev02
Thanks,
Krishna
-----Original Message-----
From: Alexey Ivanov
Sent: Tuesday, November 27, 2018 5:18 PM
To: Sergey Bylokhov <sergey.bylok...@oracle.com>; Krishna Addepalli
<krishna.addepa...@oracle.com>; awt-dev <awt-dev@openjdk.java.net>
Subject: Re: <AWT Dev> [12]RFR: JDK-8213048: Invalid use of HTML5 in java.awt
files
Hi Krishna,
On 26/11/2018 22:13, Sergey Bylokhov wrote:
Hi, Krishna.
I added the closing "</li>" tag, since it wouldn't hurt and the tool
was reporting it.
But this tag is unused in our code, is not required by html5 and it
was not reported in the bug report.
So I suggest to do not add them.
I agree with Sergey. Closing tag for <li> is not required. Adding it generates noise
in code review: 2 out of 4 files do not contain tables at all. If the closing </li>
tag is to be included, it should rather be done under a separate bugid.
As for the style spec, thanks for your suggestion, I have moved it to
the style block in the head section.
Can we to drop them completely? are these custom styles really necessary?
I think we can drop all these properties at least for DesktopProperties.html.
The first table – The Standard Blocking Matrix — in Modality.html could
benefit from centring the text. On the other hand, it remains readable
and understandable when using the default left alignment.
You could also remove commented out <center> and </center> around the table:
210 <!-- <center> -->
250 <!-- </center> -->
The redundant <p> </p> at lines 209 and 211 can safely be removed.
(Empty paragraphs are ignored in HTML anyway.)
The second table in Examples is there for no particular reason. I think
presenting the examples in a nested list would be clearer than using a
table where the second column is the example number.
It could be presented as:
<ul>
<li><b>Example 1<b>
<ol>
<li>...
</ol>
<li><b>Example 2<b>
...
</ul>
Isn't it better?
Modality.html extensively uses underlined text. It should rather be bold
or italic. Underline is reserved for hyperlinks and it's not recommended
to use underline for any other purpose. Shall I file a bug to clean this up?
DesktopProperties.html for some reason uses Javadoc {@code ...} and
{@link ...} formatting, it does not make sense in plain HTML. Is it yet
another bug?
