Hi Alexey, Thanks for your suggestions. I have already committed the changes. Can we address this in a new bug?
Thanks, Krishna > On 29-Nov-2018, at 5:15 PM, Alexey Ivanov <alexey.iva...@oracle.com> wrote: > > 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? >> >
