RFR: JDK-8228554: Accessibility errors in jdwp-protocol.html

[David Holmes]

Hi Serguei,

On 28/08/2019 5:15 pm, serguei.spitsyn at oracle.com wrote:
> Hi Alex,
> 
> Thank you for the update!
> 
> The most interesting case of a table with a multilevel indent is:
> http://cr.openjdk.java.net/~amenkov/jdk14/jdwp-protocol_accessibility/2/jdwp-protocol.html#JDWP_EventRequest 
> 
> 
> I still have a doubt as new variant looks a little less 
> clear/comprehensive.
> What do you think?

I think the new version looks fine - the extra cell boundary markers in 
the original do not add anything to the clarity IMHO. I find both forms 
equally difficult to understand.

Cheers,
David


> Thanks,
> Serguei
> 
> 
> 
> On 8/26/19 16:44, Alex Menkov wrote:
>> Ok.
>>
>> Updated webrev:
>> http://cr.openjdk.java.net/~amenkov/jdk14/jdwp-protocol_accessibility/webrev.2/ 
>>
>>
>> The difference vs v.1 is:
>> - ErrorSetNode.java - added 'style="width: 20%"' for the 1st column;
>> - ConstantSetNode.java - fixed width of the 1st column (20% -> 30%)
>>
>> generated doc:
>> http://cr.openjdk.java.net/~amenkov/jdk14/jdwp-protocol_accessibility/2/jdwp-protocol.html 
>>
>>
>> --alex
>>
>> On 08/26/2019 16:00, David Holmes wrote:
>>> On 27/08/2019 8:47 am, Alex Menkov wrote:
>>>> On 08/26/2019 14:54, David Holmes wrote:
>>>>> On 27/08/2019 3:01 am, Alex Menkov wrote:
>>>>>> Hi Serguei,
>>>>>>
>>>>>> The change is intentional - it seems to me that there were too 
>>>>>> many borders in the struct description tables. I thought about 
>>>>>> removing some of them (or making them thiner or changing color to 
>>>>>> gray).
>>>>>> I don't think absence of the lines makes comprehension of the 
>>>>>> structures harder.
>>>>>
>>>>> I like the new look - especially now we have proper headers and no 
>>>>> more strange looking empty cells!
>>>>>
>>>>> My only suggestion is to make the first column of each table the 
>>>>> same width (were possible) so that the tables line up better - 
>>>>> specifically the "Error Data" table's "Value" column should be the 
>>>>> same width as the "Reply Data" table's "Type" column.
>>>>
>>>> Maybe then make 1st column of "Error Data" the same width as (Type + 
>>>> Name) columns in Out Data/Reply Data?
>>>
>>> That would not look very good IMHO.
>>>
>>> David
>>> -----
>>>
>>>> Then "Description" column in all tables will be 65%.
>>>>
>>>> BTW just discovered at error in Constants tables - they have column 
>>>> 20%, 5% and 65% - going to update the 2st column to be 30%
>>>>
>>>> --alex
>>>>
>>>>>
>>>>> Thanks,
>>>>> David
>>>>>
>>>>>> --alex
>>>>>>
>>>>>> On 08/26/2019 00:58, serguei.spitsyn at oracle.com wrote:
>>>>>>> Hi Alex,
>>>>>>>
>>>>>>> I see one issue with new table format.
>>>>>>> For instance look at the table for "DisposeObjects Command (14)".
>>>>>>> Even a better example is "RedefineClasses Command (18)".
>>>>>>> In the old tables the indentation was highlighted with the 
>>>>>>> vertical lines.
>>>>>>> It is missed in your version.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Serguei
>>>>>>>
>>>>>>>
>>>>>>> On 8/23/19 16:54, Alex Menkov wrote:
>>>>>>>> Hi all,
>>>>>>>>
>>>>>>>> Please review the fix for
>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8228554
>>>>>>>>
>>>>>>>> webrev: 
>>>>>>>> http://cr.openjdk.java.net/~amenkov/jdk14/jdwp-protocol_accessibility/webrev/ 
>>>>>>>>
>>>>>>>>
>>>>>>>> generated docs:
>>>>>>>> old: 
>>>>>>>> http://cr.openjdk.java.net/~amenkov/jdk14/jdwp-protocol_accessibility/0/jdwp-protocol.html 
>>>>>>>>
>>>>>>>> new: 
>>>>>>>> http://cr.openjdk.java.net/~amenkov/jdk14/jdwp-protocol_accessibility/1/jdwp-protocol.html 
>>>>>>>>
>>>>>>>>
>>>>>>>> specdiff:
>>>>>>>> http://cr.openjdk.java.net/~amenkov/jdk14/jdwp-protocol_accessibility/spectdiff/diff.html 
>>>>>>>>
>>>>>>>>
>>>>>>>> Summary:
>>>>>>>> - "content outside of a region" issues:
>>>>>>>>   - <div role="banner"> replaced with with <header>,
>>>>>>>>   - <div role="main"> replaced with <main>;
>>>>>>>> - table issues:
>>>>>>>>   - added column headers to all tables;
>>>>>>>>   - for every row specified row header;
>>>>>>>>   - indentation with table "colspan" reimplemented by using CSS.
>>>>>>>>
>>>>>>>> --alex
>>>>>>>
>