Re: [Rev 03] RFR: 8208169: can not print selected pages of web page

2020-06-15 Thread Kevin Rushforth 
On Mon, 15 Jun 2020 05:22:48 GMT, Bhawesh Choudhary 
 wrote:

>> Print function of WebEngine.java ignores page range setting and prints given 
>> number of pages starting from first page,
>> which is the root cause of this issue. To fix it, put check for page ranges 
>> and if it available, use it for printing
>> pages otherwise print all pages as usual.
>
> Bhawesh Choudhary has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   cleanup unused variable

Both the fix and the test look good. I left a couple minor formatting / style 
comments.

modules/javafx.web/src/main/java/javafx/scene/web/WebEngine.java line 1615:

> 1614: JobSettings jobSettings = job.getJobSettings();
> 1615: if(jobSettings.getPageRanges() != null) {
> 1616: PageRange[] pageRanges = jobSettings.getPageRanges();

Minor: space after `if`.

tests/manual/printing/PrintPageRangeTest.java line 83:

> 82: htmlStringBuilder.append("");
> 83: for(int i = 0; i < 500; ++i) {
> 84: htmlStringBuilder.append(" HTML Line No. ");

Minor: space after `for`

tests/manual/printing/PrintPageRangeTest.java line 122:

> 121:
> 122: private void SetMessage(String msg) {
> 123: bottomMessageLabel.setText(msg);

Minor: usual naming convention is for an initial lower-case letter.

-

PR: https://git.openjdk.java.net/jfx/pull/222

Re: [Rev 03] RFR: 8208169: can not print selected pages of web page

2020-06-15 Thread Phil Race 
On Mon, 15 Jun 2020 05:22:48 GMT, Bhawesh Choudhary 
 wrote:

>> Print function of WebEngine.java ignores page range setting and prints given 
>> number of pages starting from first page,
>> which is the root cause of this issue. To fix it, put check for page ranges 
>> and if it available, use it for printing
>> pages otherwise print all pages as usual.
>
> Bhawesh Choudhary has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   cleanup unused variable

Marked as reviewed by prr (Reviewer).

-

PR: https://git.openjdk.java.net/jfx/pull/222

Re: [Rev 03] RFR: 8208169: can not print selected pages of web page

2020-06-14 Thread Bhawesh Choudhary 
> Print function of WebEngine.java ignores page range setting and prints given 
> number of pages starting from first page,
> which is the root cause of this issue. To fix it, put check for page ranges 
> and if it available, use it for printing
> pages otherwise print all pages as usual.

Bhawesh Choudhary has updated the pull request incrementally with one 
additional commit since the last revision:

  cleanup unused variable

-

Changes:
  - all: https://git.openjdk.java.net/jfx/pull/222/files
  - new: https://git.openjdk.java.net/jfx/pull/222/files/4d973a33..717dac0f

Webrevs:
 - full: https://webrevs.openjdk.java.net/jfx/222/webrev.03
 - incr: https://webrevs.openjdk.java.net/jfx/222/webrev.02-03

  Stats: 2 lines in 1 file changed: 0 ins; 2 del; 0 mod
  Patch: https://git.openjdk.java.net/jfx/pull/222.diff
  Fetch: git fetch https://git.openjdk.java.net/jfx pull/222/head:pull/222

PR: https://git.openjdk.java.net/jfx/pull/222