Re: [9] Review Request: 8180889 Cleanup of javadoc in java.datatransfer module

2017-06-06 Thread Alexander Zvegintsev

Looks good to me.

Thanks,
Alexander.

On 25/05/2017 06:54, Sergey Bylokhov wrote:

Hi, Phil.
Thank you for a review, the fix is updated:
Webrev can be found at: http://cr.openjdk.java.net/~serb/8180889/webrev.01
Specdiff: 
http://cr.openjdk.java.net/~serb/8180889/specdiff.01/java/awt/datatransfer/package-summary.html


Note that I have updated the text about Oracle's implementation and 
moved it to @implNote tag:

http://cr.openjdk.java.net/~serb/8180889/specdiff.01/java/awt/datatransfer/DataFlavor-report.html#method:getTextPlainUnicodeFlavor()
So I'll file a ccc after the technical review.

- philip.r...@oracle.com wrote:
>
I don't know why it was necessary to update the non-API classes here, 
and I also see some lines reformatted without any actual changes, even 
white space fixes but apart from adding to noise in the change they 
are OK. also this is not strictly doc since you cleaned up (not just 
reordered) some import statements so I added noreg-cleanup to the bug 
labels. A few nits : DataFlavor.java 104 * to Use Drag and Drop and 
Data Transfer, section in

105 * Java Tutorial. in Java Tutorial -> in the Java Tutorial
483 * successfully loaded, then an {@code ClassNotFoundException} is 
an -> a
594 * Sun's implementation for Microsoft Windows uses the encoding 595 
* {@code utf-16le}. 596 * Sun's implementation for Solaris and Linux 
uses the encoding Sun -> Oracle ?? And what about MacOS ?
1249 * @return {@code true} if the {@code DataFlavor} specified 
represents a 1250 * List of File objects 1250 should perhaps be : 
{@code java.util.List} of {@code java.io.File} objects -phil.

> On 05/23/2017 07:19 PM, Sergey Bylokhov wrote:
>

Hello,
Please review the fix for jdk9.

Bug:https://bugs.openjdk.java.net/browse/JDK-8180889
Webrev can be found at:http://cr.openjdk.java.net/~serb/8180889/webrev.00

Specdiff:http://cr.openjdk.java.net/~serb/8180889/specdiff.00/java/awt/datatransfer/package-summary.html


In jdk9 a lots of specifications and javadocs were updated, but it seems 
that java.datatransfer module was missed, because the client team cleanup only 
the java.desktop module.

In this fix the only javadoc is updated and the next rules were applied:
  -  should be replaced by {@tag }
  - @deprecated tag should have some text
  - 80 column limit
  - description of the class/method/field should be followed by dot
  - @param, @return should not end with a dot, except a case when more than 
one sentences are used
  - empty line after description/before the first tag was added
  - unnecessary empty lines were removed
  - sets of spaces in the middle of text were deleted
  - @param, @throws, @return should be aligned, to be more readable
  - unnecessary imports should be removed
  - the "null"/"true"/"false"/"this" should be wrapped in {@code } when 
necessary
  - the order of different tags were unified across the package

The specdiff is provided and the most visible changes are:

http://cr.openjdk.java.net/~serb/8180889/specdiff.00/java/awt/datatransfer/DataFlavor-report.html#method:normalizeMimeTypeParameter(java.lang.String,%20java.lang.String)

http://cr.openjdk.java.net/~serb/8180889/specdiff.00/java/awt/datatransfer/DataFlavor-report.html#method:normalizeMimeType(java.lang.String)
where the text about deprecation was moved to the @deprecated tag.


>




Re: [10] Review request for JDK-8180370: Characters are skipped on input of Korean text on OS X

2017-06-06 Thread Sergey Bylokhov
Perhaps the test can be simplified further(some flags can be removed as well as 
the number of try/catch may be minimized), but this version loos fine.
Thanks.

- sreeprakas...@oracle.com wrote:

> Thanks for the inputs Sergey.
> 
> I have changed the test case to use CountDownLatch and Swing
> components. I have also moved the test location from awt/TextField to
> swing/JTextField
> 
> Updated Webrev :
> http://cr.openjdk.java.net/~rpatil/8180370/webrev.03/
> 
> I have also tested out the cases that you had mentioned (JDK-8148555,
> JDK-8132503,JDK-8073008 & JDK-8068283) and they all worked fine.
> 
> Regards,
> Sreeprakash
> 
> -Original Message-
> From: Sergey Bylokhov 
> Sent: Tuesday, June 6, 2017 9:57 AM
> To: Sreeprakash Sreedharan 
> Cc: Philip Race ; awt-dev@openjdk.java.net
> Subject: Re:  [10] Review request for JDK-8180370: Characters
> are skipped on input of Korean text on OS X
> 
> Hi,
> I suggest to check the Swing component first because they have more
> priority, so the swing test will be useful.
> A few notes about the fix:
>  - Can you please double check that the cases described in
> JDK-8148555, JDK-8132503,JDK-8073008,JDK-8068283 still works as
> expected.  We had some regressions here so it will be useful to double
> check.
>  - Its possible to tweak the test a little bit, and use CountDownLatch
> instead of Thread.sleep + interrupt()+TimerTask +Timer. Just await()
> the main method and countDown() then the user press the button. In
> this case you can move all logic from time to the main method. I guess
> this will simplify the test.
>  - Note that in case of swing you will need to call validateInput() on
> EDT.
> 
> > 
> > As the term "Special character" was ambiguous, I have renamed the
> test case and the appropriate comments and also removed some swing
> dependencies.
> > Updated Webrev :
> http://cr.openjdk.java.net/~rpatil/8180370/webrev.02/
> > 
> > Also, I have written the test case for the AWT component (TextField)
> only.
> > Is it required to write a duplicate test case for the equivalent
> swing component (JTextField) ?
> > 
> > Regards,
> > Sreeprakash
> > 
> > -Original Message-
> > From: Phil Race
> > Sent: Saturday, June 3, 2017 2:29 AM
> > To: Sreeprakash Sreedharan ; 
> > awt-dev@openjdk.java.net
> > Subject: Re:  [10] Review request for JDK-8180370:
> Characters 
> > are skipped on input of Korean text on OS X
> > 
> > OK .. although I think this may need bake time before being
> backported.
> > but since I suppose hardly anyone is using or testing 10 at the
> moment that is going to be tricky.
> > 
> > -phil.
> > 
> > On 06/02/2017 01:47 PM, Sreeprakash Sreedharan wrote:
> >> Thanks for the inputs Phil.
> >> 
> >> I have updated the code and the comments.
> >> 
> >> Updated Webrev : 
> >> http://cr.openjdk.java.net/~rpatil/8180370/webrev.01/
> >> 
> >> The issue was that any non-alpha character(numbers and symbols)
> typed immediately after a Korean character was getting skipped.
> >> The non-alpha character will come if you type again.
> >> For example, an exclamation mark (!) entered after the Korean
> character for q will not show up.
> >> However, if you type it again it will appear. But instead of having
> 2 exclamation marks we have just one.
> >> 
> >> I had tested it out with 2-Set Korean, Wubi Xing (Chinese) and ABC
> - Extended keyboard layouts on mac with different input methods .
> >> 
> >> I had run all the manual and automatic regression tests for all
> text based controls in awt (like TextField, TextArea etc) and swing
> (like JTextField, JTextArea, JTextPane, JEditorPane etc) and did not
> find any issues.
> >> 
> >> Thanks,
> >> Sreeprakash
> >> 
> >> 
> >> From: Phil Race
> >> Sent: Friday, June 2, 2017 9:44 PM
> >> To: Sreeprakash Sreedharan ; 
> >> awt-dev@openjdk.java.net
> >> Subject: Re:  [10] Review request for JDK-8180370: 
> >> Characters are skipped on input of Korean text on OS X
> >> 
> >> I am not familiar with this code but I have a few comments anyway
> >> 
> >> 1. I dislike cluttering the source with bug ids. If we did that for
> every fix
> >> quite soon the source code would be a mess of semi-random
> numbers.
> >> Anyone who really wants to know when this change was made has
> the 
> >> history
> >> 
> >> 2. if is not a function. So "if(" -> "if ("
> >> 
> >> 3. When a marked text -> When marked text
> >> 
> >> 4. "!" is not a "special character" .. its quite ordinary .. so
> what do you mean ?
> >> 
> >> 5. What testing have you done to make sure no other cases are
> broken by this change ?
> >> The new test is manual and I'd bet that most tests that might
> cover this are manual
> >> I'd expect to hear that you have tested different input
> scenarios such as a couple
> >> of input methods/locales, and AWT and Swing input with a
> representative set of
> >> input as well as running 

Re: [10] Review request for JDK-8180370: Characters are skipped on input of Korean text on OS X

2017-06-06 Thread Sreeprakash Sreedharan
Thanks for the inputs Sergey.

I have changed the test case to use CountDownLatch and Swing components. I have 
also moved the test location from awt/TextField to swing/JTextField

Updated Webrev : http://cr.openjdk.java.net/~rpatil/8180370/webrev.03/

I have also tested out the cases that you had mentioned (JDK-8148555, 
JDK-8132503,JDK-8073008 & JDK-8068283) and they all worked fine.

Regards,
Sreeprakash

-Original Message-
From: Sergey Bylokhov 
Sent: Tuesday, June 6, 2017 9:57 AM
To: Sreeprakash Sreedharan 
Cc: Philip Race ; awt-dev@openjdk.java.net
Subject: Re:  [10] Review request for JDK-8180370: Characters are 
skipped on input of Korean text on OS X

Hi,
I suggest to check the Swing component first because they have more priority, 
so the swing test will be useful.
A few notes about the fix:
 - Can you please double check that the cases described in JDK-8148555, 
JDK-8132503,JDK-8073008,JDK-8068283 still works as expected.  We had some 
regressions here so it will be useful to double check.
 - Its possible to tweak the test a little bit, and use CountDownLatch instead 
of Thread.sleep + interrupt()+TimerTask +Timer. Just await() the main method 
and countDown() then the user press the button. In this case you can move all 
logic from time to the main method. I guess this will simplify the test.
 - Note that in case of swing you will need to call validateInput() on EDT.

> 
> As the term "Special character" was ambiguous, I have renamed the test case 
> and the appropriate comments and also removed some swing dependencies.
> Updated Webrev : http://cr.openjdk.java.net/~rpatil/8180370/webrev.02/
> 
> Also, I have written the test case for the AWT component (TextField) only.
> Is it required to write a duplicate test case for the equivalent swing 
> component (JTextField) ?
> 
> Regards,
> Sreeprakash
> 
> -Original Message-
> From: Phil Race
> Sent: Saturday, June 3, 2017 2:29 AM
> To: Sreeprakash Sreedharan ; 
> awt-dev@openjdk.java.net
> Subject: Re:  [10] Review request for JDK-8180370: Characters 
> are skipped on input of Korean text on OS X
> 
> OK .. although I think this may need bake time before being backported.
> but since I suppose hardly anyone is using or testing 10 at the moment that 
> is going to be tricky.
> 
> -phil.
> 
> On 06/02/2017 01:47 PM, Sreeprakash Sreedharan wrote:
>> Thanks for the inputs Phil.
>> 
>> I have updated the code and the comments.
>> 
>> Updated Webrev : 
>> http://cr.openjdk.java.net/~rpatil/8180370/webrev.01/
>> 
>> The issue was that any non-alpha character(numbers and symbols) typed 
>> immediately after a Korean character was getting skipped.
>> The non-alpha character will come if you type again.
>> For example, an exclamation mark (!) entered after the Korean character for 
>> q will not show up.
>> However, if you type it again it will appear. But instead of having 2 
>> exclamation marks we have just one.
>> 
>> I had tested it out with 2-Set Korean, Wubi Xing (Chinese) and ABC - 
>> Extended keyboard layouts on mac with different input methods .
>> 
>> I had run all the manual and automatic regression tests for all text based 
>> controls in awt (like TextField, TextArea etc) and swing (like JTextField, 
>> JTextArea, JTextPane, JEditorPane etc) and did not find any issues.
>> 
>> Thanks,
>> Sreeprakash
>> 
>> 
>> From: Phil Race
>> Sent: Friday, June 2, 2017 9:44 PM
>> To: Sreeprakash Sreedharan ; 
>> awt-dev@openjdk.java.net
>> Subject: Re:  [10] Review request for JDK-8180370: 
>> Characters are skipped on input of Korean text on OS X
>> 
>> I am not familiar with this code but I have a few comments anyway
>> 
>> 1. I dislike cluttering the source with bug ids. If we did that for every fix
>> quite soon the source code would be a mess of semi-random numbers.
>> Anyone who really wants to know when this change was made has the 
>> history
>> 
>> 2. if is not a function. So "if(" -> "if ("
>> 
>> 3. When a marked text -> When marked text
>> 
>> 4. "!" is not a "special character" .. its quite ordinary .. so what do you 
>> mean ?
>> 
>> 5. What testing have you done to make sure no other cases are broken by this 
>> change ?
>> The new test is manual and I'd bet that most tests that might cover this 
>> are manual
>> I'd expect to hear that you have tested different input scenarios such 
>> as a couple
>> of input methods/locales, and AWT and Swing input with a representative 
>> set of
>> input as well as running all the relevant regression tests.
>> 
>> -phil.
>> On 06/02/2017 07:48 AM, Sreeprakash Sreedharan wrote:
>> Hi All,
>> 
>> Kindly review the fix for JDK10.
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8180370
>> Webrev: http://cr.openjdk.java.net/~rpatil/8180370/webrev.00/
>> Issue: Special characters (like !,/\<> ) were getting skipped when 
>> immediately entered after a marked text on 

Re: [9] Review Request: 8180326 Update the tables in java.desktop to be HTML-5 friendly

2017-06-06 Thread Alexander Zvegintsev

Hi Sergey,

Why do we omitting closing th tag?

e.g.

+ * Metal's system color mapping
+ * 
+ * 
+ * Key
+ * Value
+ * 

I know that HTML parsers are usually forgiving such things. But 
sometimes it may make thing worse:


https://stackoverflow.com/questions/7125354/what-are-the-actual-problems-of-not-closing-tags-and-attributes-in-html/7135378#7135378

Thanks,
Alexander.

On 05/06/2017 06:23, Sergey Bylokhov wrote:

If there are no objections I'll change the target ws from dev to client, to 
minimize the merges between some other javadoc fixes.

- sergey.bylok...@oracle.com wrote:


Hello.
Here is an updated version where most of the caption are visible.
Bug: https://bugs.openjdk.java.net/browse/JDK-8180326
Webrev can be found at:
http://cr.openjdk.java.net/~serb/8180326/webrev.02/
Specdiff:
http://cr.openjdk.java.net/~serb/8180326/specdiff.02/overview-summary.html

You can use search to check the changes in some specific class:
Old docs:
http://cr.openjdk.java.net/~serb/8180326/api_old.02/overview-summary.html
New docs:
http://cr.openjdk.java.net/~serb/8180326/api.02/overview-summary.html


- jonathan.gibb...@oracle.com wrote:


Phil,

I have no evidence one way or the other whether screen readers pay
attention
to undisplayed or invisible captions. It seemed safest to assume

that

they would
read a visible caption, and that we should head in that general
direction.

-- Jon


On 05/17/2017 11:58 AM, Phil Race wrote:

And PS I was not saying anything to contradict

tables should not have a summary attribute and should have a

caption.

However that the docs I read on the web did seem to imply that
summary was very much intended for ATs but it was not at all

clear

this

is the point of caption. I'm sure they can read it, but I don't

get

how making
it visible matters to them so how it making it visible relates to
accessibility
requirements is not an obvious connection to me. So why do we

have

to make it visible for ATs ?

-phil.

On 05/17/2017 11:54 AM, Phil Race wrote:

I will leave the decision on whether to do that now up to Sergey
although
it seems all he has to do here is remove "invisible".
Many of the "summary" ones had wrong or misleading text but they
seem to have been all fixed.

I'd want to see what the new HTML looks like with a visible

title

of

course ..

-phil.

On 05/17/2017 11:52 AM, Jonathan Gibbons wrote:

Phil,

The bottom line is that in the JDK docs, tables should not have

a

summary attribute and should have a caption. This comes down to
accessibility requirements, where we are slowly raising the bar

on

our docs, to be in accordance with Oracle's guidelines.

Hiding the caption (style="display:none") is an interim measure

we

have been using during the HTML 5 updates, especially in cases

where

the person doing the markup changes did not know enough to

create

an

appropriate caption that should be displayed. In time, we should
locate and update all table captions (in our standard docs

bundle)

that are not being displayed such that the text is both

appropriate

and visible. If you guys want to do that as part of this

update,

go

ahead. FWIW, that is what we did for the java.xml module in the

jaxp

repo ... pretty much all tables there now have a reasonable,

visible

caption.

-- Jon



On 05/17/2017 11:19 AM, Phil Race wrote:

I am not sure we are using the summary in a way that makes it
worthwhile.
As you noted in the other mail
"The summary attribute was used to give a more descriptive

value

of the contents of the table.   A caption is more like a

title"

The values I see are more like a title and as you say that is

not

the idea. See the example here

https://www.w3.org/TR/WCAG20-TECHS/H73.html

Caption sounds like a title so it might actually be more
appropriate than summary
for the text we have except that its not clear why we'd want

it

to

be visible when we were fine without.

But being there and invisible may be pointless unless screen
readers look for it even if invisible.

But if its not doing any harm I guess we can leave it as

proposed

I still need to look at the rest of the changes.

-phil.

On 05/12/2017 05:11 PM, Jonathan Gibbons wrote:

Sergey,

FWIW, the invisible caption should be regarded as a temporary
solution, until content authors can review/update the text of

the

caption and make it visible.

The general guideline in this conversion work has been to

avoid

changing the visible text of the specification, and captions

fall

into a grey area of whether the text is significant/normative

or

not.  Hence the temporary step to make them not displayed for

now.

-- Jon

On 05/12/2017 05:00 PM, Sergey Bylokhov wrote:

The "summary" is unsupported by the HTML5 and we replace it

by

invisible caption.
These new styles are located in the stylesheet.css in the

root

of

the JavaDoc api folder, so I assume these styles should be

used

by others as well.
They were added by this fix: