Quantcast

<Swing Dev> [9] Review Request: 8177450: javax.swing.text.html.parser.Parser parseScript ignores a character after commend end

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

<Swing Dev> [9] Review Request: 8177450: javax.swing.text.html.parser.Parser parseScript ignores a character after commend end

mikhail cherkasov
Hello,

Could you please review the fix for jdk9?

The problem is that Parser reads extra symbol after comment or script tag and missed the symbol.
I added 'continue' after those tags, so it goes to beginning of Parser while loop and process the symbol.
  
Bug: https://bugs.openjdk.java.net/browse/JDK-8177450
Webrev: http://cr.openjdk.java.net/~mcherkas/8177450/9/webrev/

Thanks,
Mikhail. 


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: <Swing Dev> [9] Review Request: 8177450: javax.swing.text.html.parser.Parser parseScript ignores a character after commend end

Alexandr Scherbatiy

The fix looks good to me.

Thanks,
Alexandr.

On 3/31/2017 2:28 PM, Mikhail Cherkasov wrote:
Hello,

Could you please review the fix for jdk9?

The problem is that Parser reads extra symbol after comment or script tag and missed the symbol.
I added 'continue' after those tags, so it goes to beginning of Parser while loop and process the symbol.
  
Bug: https://bugs.openjdk.java.net/browse/JDK-8177450
Webrev: http://cr.openjdk.java.net/~mcherkas/8177450/9/webrev/

Thanks,
Mikhail. 



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: <Swing Dev> [9] Review Request: 8177450: javax.swing.text.html.parser.Parser parseScript ignores a character after commend end

Philip Race
In reply to this post by mikhail cherkasov
2122             if(i > 0) {

should be "if (i > 0) {"

Can you comment on what tests have been run to ensure this does not regress anything else ?

-phil.


On 03/31/2017 04:28 AM, Mikhail Cherkasov wrote:
Hello,

Could you please review the fix for jdk9?

The problem is that Parser reads extra symbol after comment or script tag and missed the symbol.
I added 'continue' after those tags, so it goes to beginning of Parser while loop and process the symbol.
  
Bug: https://bugs.openjdk.java.net/browse/JDK-8177450
Webrev: http://cr.openjdk.java.net/~mcherkas/8177450/9/webrev/

Thanks,
Mikhail. 



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: <Swing Dev> [9] Review Request: 8177450: javax.swing.text.html.parser.Parser parseScript ignores a character after commend end

mikhail cherkasov
Hi Phil,

On 4/4/2017 9:13 PM, Phil Race wrote:
2122             if(i > 0) {

should be "if (i > 0) {"
fixed: http://cr.openjdk.java.net/~mcherkas/8177450/9/webrev.01/


Can you comment on what tests have been run to ensure this does not regress anything else ?
I run tests from " test/javax/swing/text/html/parser/" and from "test/closed/javax/swing/text/html/parser/"
no new failures, also I run swing JCK tests and again no new failures.


-phil.


On 03/31/2017 04:28 AM, Mikhail Cherkasov wrote:
Hello,

Could you please review the fix for jdk9?

The problem is that Parser reads extra symbol after comment or script tag and missed the symbol.
I added 'continue' after those tags, so it goes to beginning of Parser while loop and process the symbol.
  
Bug: https://bugs.openjdk.java.net/browse/JDK-8177450
Webrev: http://cr.openjdk.java.net/~mcherkas/8177450/9/webrev/

Thanks,
Mikhail. 




Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: <Swing Dev> [9] Review Request: 8177450: javax.swing.text.html.parser.Parser parseScript ignores a character after commend end

Philip Race
Ok. Sorry there is one more thing.
I know its a sustaining habit but we've previously advised to stop
using the bug id as the test name. Instead use a short name which
is relevant to what is being tested. eg 'HtmlCommentParseTest.java'
The bug ID is in the @bug tag for anyone who needs to know it.

-phil.

On 04/05/2017 03:36 AM, Mikhail Cherkasov wrote:
Hi Phil,

On 4/4/2017 9:13 PM, Phil Race wrote:
2122             if(i > 0) {

should be "if (i > 0) {"
fixed: http://cr.openjdk.java.net/~mcherkas/8177450/9/webrev.01/


Can you comment on what tests have been run to ensure this does not regress anything else ?
I run tests from " test/javax/swing/text/html/parser/" and from "test/closed/javax/swing/text/html/parser/"
no new failures, also I run swing JCK tests and again no new failures.


-phil.


On 03/31/2017 04:28 AM, Mikhail Cherkasov wrote:
Hello,

Could you please review the fix for jdk9?

The problem is that Parser reads extra symbol after comment or script tag and missed the symbol.
I added 'continue' after those tags, so it goes to beginning of Parser while loop and process the symbol.
  
Bug: https://bugs.openjdk.java.net/browse/JDK-8177450
Webrev: http://cr.openjdk.java.net/~mcherkas/8177450/9/webrev/

Thanks,
Mikhail. 





Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: <Swing Dev> [9] Review Request: 8177450: javax.swing.text.html.parser.Parser parseScript ignores a character after commend end

mikhail cherkasov
I re-named the test: http://cr.openjdk.java.net/~mcherkas/8177450/9/webrev.02

On 4/5/2017 11:46 PM, Phil Race wrote:
Ok. Sorry there is one more thing.
I know its a sustaining habit but we've previously advised to stop
using the bug id as the test name. Instead use a short name which
is relevant to what is being tested. eg 'HtmlCommentParseTest.java'
The bug ID is in the @bug tag for anyone who needs to know it.

-phil.

On 04/05/2017 03:36 AM, Mikhail Cherkasov wrote:
Hi Phil,

On 4/4/2017 9:13 PM, Phil Race wrote:
2122             if(i > 0) {

should be "if (i > 0) {"
fixed: http://cr.openjdk.java.net/~mcherkas/8177450/9/webrev.01/


Can you comment on what tests have been run to ensure this does not regress anything else ?
I run tests from " test/javax/swing/text/html/parser/" and from "test/closed/javax/swing/text/html/parser/"
no new failures, also I run swing JCK tests and again no new failures.


-phil.


On 03/31/2017 04:28 AM, Mikhail Cherkasov wrote:
Hello,

Could you please review the fix for jdk9?

The problem is that Parser reads extra symbol after comment or script tag and missed the symbol.
I added 'continue' after those tags, so it goes to beginning of Parser while loop and process the symbol.
  
Bug: https://bugs.openjdk.java.net/browse/JDK-8177450
Webrev: http://cr.openjdk.java.net/~mcherkas/8177450/9/webrev/

Thanks,
Mikhail. 






Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: <Swing Dev> [9] Review Request: 8177450: javax.swing.text.html.parser.Parser parseScript ignores a character after commend end

mikhail cherkasov
Hi Phil,

Do you have any other comments about the fix?

Thanks,
Mikhail.

On 4/6/2017 10:59 AM, Mikhail Cherkasov wrote:
I re-named the test: http://cr.openjdk.java.net/~mcherkas/8177450/9/webrev.02

On 4/5/2017 11:46 PM, Phil Race wrote:
Ok. Sorry there is one more thing.
I know its a sustaining habit but we've previously advised to stop
using the bug id as the test name. Instead use a short name which
is relevant to what is being tested. eg 'HtmlCommentParseTest.java'
The bug ID is in the @bug tag for anyone who needs to know it.

-phil.

On 04/05/2017 03:36 AM, Mikhail Cherkasov wrote:
Hi Phil,

On 4/4/2017 9:13 PM, Phil Race wrote:
2122             if(i > 0) {

should be "if (i > 0) {"
fixed: http://cr.openjdk.java.net/~mcherkas/8177450/9/webrev.01/


Can you comment on what tests have been run to ensure this does not regress anything else ?
I run tests from " test/javax/swing/text/html/parser/" and from "test/closed/javax/swing/text/html/parser/"
no new failures, also I run swing JCK tests and again no new failures.


-phil.


On 03/31/2017 04:28 AM, Mikhail Cherkasov wrote:
Hello,

Could you please review the fix for jdk9?

The problem is that Parser reads extra symbol after comment or script tag and missed the symbol.
I added 'continue' after those tags, so it goes to beginning of Parser while loop and process the symbol.
  
Bug: https://bugs.openjdk.java.net/browse/JDK-8177450
Webrev: http://cr.openjdk.java.net/~mcherkas/8177450/9/webrev/

Thanks,
Mikhail. 







Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: <Swing Dev> [9] Review Request: 8177450: javax.swing.text.html.parser.Parser parseScript ignores a character after commend end

Philip Race
No more comments. That's fine.

-phil.

On 4/9/17, 11:24 AM, Mikhail Cherkasov wrote:
Hi Phil,

Do you have any other comments about the fix?

Thanks,
Mikhail.

On 4/6/2017 10:59 AM, Mikhail Cherkasov wrote:
I re-named the test: http://cr.openjdk.java.net/~mcherkas/8177450/9/webrev.02

On 4/5/2017 11:46 PM, Phil Race wrote:
Ok. Sorry there is one more thing.
I know its a sustaining habit but we've previously advised to stop
using the bug id as the test name. Instead use a short name which
is relevant to what is being tested. eg 'HtmlCommentParseTest.java'
The bug ID is in the @bug tag for anyone who needs to know it.

-phil.

On 04/05/2017 03:36 AM, Mikhail Cherkasov wrote:
Hi Phil,

On 4/4/2017 9:13 PM, Phil Race wrote:
2122             if(i > 0) {

should be "if (i > 0) {"
fixed: http://cr.openjdk.java.net/~mcherkas/8177450/9/webrev.01/


Can you comment on what tests have been run to ensure this does not regress anything else ?
I run tests from " test/javax/swing/text/html/parser/" and from "test/closed/javax/swing/text/html/parser/"
no new failures, also I run swing JCK tests and again no new failures.


-phil.


On 03/31/2017 04:28 AM, Mikhail Cherkasov wrote:
Hello,

Could you please review the fix for jdk9?

The problem is that Parser reads extra symbol after comment or script tag and missed the symbol.
I added 'continue' after those tags, so it goes to beginning of Parser while loop and process the symbol.
  
Bug: https://bugs.openjdk.java.net/browse/JDK-8177450
Webrev: http://cr.openjdk.java.net/~mcherkas/8177450/9/webrev/

Thanks,
Mikhail. 







Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: <Swing Dev> [9] Review Request: 8177450: javax.swing.text.html.parser.Parser parseScript ignores a character after commend end

mikhail cherkasov
Ok, thank you!

On 4/9/2017 9:41 PM, Philip Race wrote:
No more comments. That's fine.

-phil.

On 4/9/17, 11:24 AM, Mikhail Cherkasov wrote:
Hi Phil,

Do you have any other comments about the fix?

Thanks,
Mikhail.

On 4/6/2017 10:59 AM, Mikhail Cherkasov wrote:
I re-named the test: http://cr.openjdk.java.net/~mcherkas/8177450/9/webrev.02

On 4/5/2017 11:46 PM, Phil Race wrote:
Ok. Sorry there is one more thing.
I know its a sustaining habit but we've previously advised to stop
using the bug id as the test name. Instead use a short name which
is relevant to what is being tested. eg 'HtmlCommentParseTest.java'
The bug ID is in the @bug tag for anyone who needs to know it.

-phil.

On 04/05/2017 03:36 AM, Mikhail Cherkasov wrote:
Hi Phil,

On 4/4/2017 9:13 PM, Phil Race wrote:
2122             if(i > 0) {

should be "if (i > 0) {"
fixed: http://cr.openjdk.java.net/~mcherkas/8177450/9/webrev.01/


Can you comment on what tests have been run to ensure this does not regress anything else ?
I run tests from " test/javax/swing/text/html/parser/" and from "test/closed/javax/swing/text/html/parser/"
no new failures, also I run swing JCK tests and again no new failures.


-phil.


On 03/31/2017 04:28 AM, Mikhail Cherkasov wrote:
Hello,

Could you please review the fix for jdk9?

The problem is that Parser reads extra symbol after comment or script tag and missed the symbol.
I added 'continue' after those tags, so it goes to beginning of Parser while loop and process the symbol.
  
Bug: https://bugs.openjdk.java.net/browse/JDK-8177450
Webrev: http://cr.openjdk.java.net/~mcherkas/8177450/9/webrev/

Thanks,
Mikhail. 








Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: <Swing Dev> [9] Review Request: 8177450: javax.swing.text.html.parser.Parser parseScript ignores a character after commend end

Alexandr Scherbatiy

The fix looks good to me.

Thanks,
Alexndr.

On 4/9/2017 10:33 PM, Mikhail Cherkasov wrote:
Ok, thank you!

On 4/9/2017 9:41 PM, Philip Race wrote:
No more comments. That's fine.

-phil.

On 4/9/17, 11:24 AM, Mikhail Cherkasov wrote:
Hi Phil,

Do you have any other comments about the fix?

Thanks,
Mikhail.

On 4/6/2017 10:59 AM, Mikhail Cherkasov wrote:
I re-named the test: http://cr.openjdk.java.net/~mcherkas/8177450/9/webrev.02

On 4/5/2017 11:46 PM, Phil Race wrote:
Ok. Sorry there is one more thing.
I know its a sustaining habit but we've previously advised to stop
using the bug id as the test name. Instead use a short name which
is relevant to what is being tested. eg 'HtmlCommentParseTest.java'
The bug ID is in the @bug tag for anyone who needs to know it.

-phil.

On 04/05/2017 03:36 AM, Mikhail Cherkasov wrote:
Hi Phil,

On 4/4/2017 9:13 PM, Phil Race wrote:
2122             if(i > 0) {

should be "if (i > 0) {"
fixed: http://cr.openjdk.java.net/~mcherkas/8177450/9/webrev.01/


Can you comment on what tests have been run to ensure this does not regress anything else ?
I run tests from " test/javax/swing/text/html/parser/" and from "test/closed/javax/swing/text/html/parser/"
no new failures, also I run swing JCK tests and again no new failures.


-phil.


On 03/31/2017 04:28 AM, Mikhail Cherkasov wrote:
Hello,

Could you please review the fix for jdk9?

The problem is that Parser reads extra symbol after comment or script tag and missed the symbol.
I added 'continue' after those tags, so it goes to beginning of Parser while loop and process the symbol.
  
Bug: https://bugs.openjdk.java.net/browse/JDK-8177450
Webrev: http://cr.openjdk.java.net/~mcherkas/8177450/9/webrev/

Thanks,
Mikhail. 









Loading...