Re: [PATCH] 8161282: FXCanvas does not forward horizontal mouse scroll events to the embedded scene

[Kevin Rushforth]

Approved, assuming the changes to the wild-import are done by Alexander 
along with a correctly formatted changeset commit message.


-- Kevin


Kevin Rushforth wrote:

Uploaded the new version.

Alexander Zvegintsev will sponsor this patch, and I will review it.

My one quick comment -- in addition to the wild-card imports which 
Alexander Z caught, and said he would take care of -- is that the 
format of the commit message is not quite right. See the following 
page for formatting rules on the changeset commit message:


http://openjdk.java.net/guide/producingChangeset.html

Thanks.

-- Kevin



Alexander Nyssen wrote:
You might even take the one I attached. I just recognized I still had 
some unused imports in the manual test case (I am still not familiar 
with IntelliJ).


Regards,
Alexander





Am 15.08.2016 um 19:26 schrieb Kevin Rushforth 
>:


OK, I'll upload this revised version of the patch today.

-- Kevin


Alexander Nyssen wrote:

Hi Kevin,

please consider the following updated patch instead, which contains 
an additional null-check.


Regards
Alexander


 




Am 12.08.2016 um 16:04 schrieb Alexander Nyssen 
>:


Hi Kevin,

attached please find an initial patch for 
https://bugs.openjdk.java.net/browse/JDK-8161282.


The patch is not as minimal as I had hoped, as the 
EmbeddedSceneInterface had to be changed to differentiate between 
mouse and scroll events (while up to now, scroll events are 
handled as mouse events), but for me this seemed necessary to fix 
this issue properly. As a result, JFXPanel had to be adjusted as 
well to comply to the changes in the EmbeddedSceneInterface, while 
its behavior should not have changed.


As horizontal mouse events cannot be synthesized via 
Display.post(Event) yet (an open issue for SWT), I did not add an 
automated test, but instead added a manual one 
(FXCanvasMouseWheelEventsTest). Therefore, this patch does not 
depend on the patch I provided earlier for JDK-8160325.


Best Regards,
Alexander






=


=

Re: [PATCH] 8161282: FXCanvas does not forward horizontal mouse scroll events to the embedded scene

[Kevin Rushforth]

Uploaded the new version.

Alexander Zvegintsev will sponsor this patch, and I will review it.

My one quick comment -- in addition to the wild-card imports which 
Alexander Z caught, and said he would take care of -- is that the format 
of the commit message is not quite right. See the following page for 
formatting rules on the changeset commit message:


http://openjdk.java.net/guide/producingChangeset.html

Thanks.

-- Kevin



Alexander Nyssen wrote:
You might even take the one I attached. I just recognized I still had 
some unused imports in the manual test case (I am still not familiar 
with IntelliJ).


Regards,
Alexander





Am 15.08.2016 um 19:26 schrieb Kevin Rushforth 
>:


OK, I'll upload this revised version of the patch today.

-- Kevin


Alexander Nyssen wrote:

Hi Kevin,

please consider the following updated patch instead, which contains 
an additional null-check.


Regards
Alexander





Am 12.08.2016 um 16:04 schrieb Alexander Nyssen 
>:


Hi Kevin,

attached please find an initial patch 
for https://bugs.openjdk.java.net/browse/JDK-8161282.


The patch is not as minimal as I had hoped, as the 
EmbeddedSceneInterface had to be changed to differentiate between 
mouse and scroll events (while up to now, scroll events are handled 
as mouse events), but for me this seemed necessary to fix this 
issue properly. As a result, JFXPanel had to be adjusted as well to 
comply to the changes in the EmbeddedSceneInterface, while its 
behavior should not have changed.


As horizontal mouse events cannot be synthesized via 
Display.post(Event) yet (an open issue for SWT), I did not add an 
automated test, but instead added a manual one 
(FXCanvasMouseWheelEventsTest). Therefore, this patch does not 
depend on the patch I provided earlier for JDK-8160325.


Best Regards,
Alexander






=


=

Re: [PATCH] 8161282: FXCanvas does not forward horizontal mouse scroll events to the embedded scene

[Alexander Zvegintsev]

Hi Alexander,

The fix looks good to me, except the wild card import change in 
FXCanvas.java(there is no need for a new patch, if it is the only issue 
found)



On 8/16/16 10:24 AM, Alexander Nyssen wrote:

You might even take the one I attached. I just recognized I still had some 
unused imports in the manual test case (I am still not familiar with IntelliJ).

Regards,
Alexander




Am 15.08.2016 um 19:26 schrieb Kevin Rushforth :

OK, I'll upload this revised version of the patch today.

-- Kevin


Alexander Nyssen wrote:

Hi Kevin,

please consider the following updated patch instead, which contains an 
additional null-check.

Regards
Alexander





Am 12.08.2016 um 16:04 schrieb Alexander Nyssen >:

Hi Kevin,

attached please find an initial patch for 
https://bugs.openjdk.java.net/browse/JDK-8161282 
.

The patch is not as minimal as I had hoped, as the EmbeddedSceneInterface had 
to be changed to differentiate between mouse and scroll events (while up to 
now, scroll events are handled as mouse events), but for me this seemed 
necessary to fix this issue properly. As a result, JFXPanel had to be adjusted 
as well to comply to the changes in the EmbeddedSceneInterface, while its 
behavior should not have changed.

As horizontal mouse events cannot be synthesized via Display.post(Event) yet 
(an open issue for SWT), I did not add an automated test, but instead added a 
manual one (FXCanvasMouseWheelEventsTest). Therefore, this patch does not 
depend on the patch I provided earlier for JDK-8160325.

Best Regards,
Alexander





=


--
Thanks,
Alexander.

Re: [PATCH] 8161282: FXCanvas does not forward horizontal mouse scroll events to the embedded scene

[Alexander Nyssen]

You might even take the one I attached. I just recognized I still had some 
unused imports in the manual test case (I am still not familiar with IntelliJ).

Regards,
Alexander



> Am 15.08.2016 um 19:26 schrieb Kevin Rushforth :
> 
> OK, I'll upload this revised version of the patch today.
> 
> -- Kevin
> 
> 
> Alexander Nyssen wrote:
>> 
>> Hi Kevin,
>> 
>> please consider the following updated patch instead, which contains an 
>> additional null-check.
>> 
>> Regards
>> Alexander
>> 
>> 
>> 
>> 
>>> Am 12.08.2016 um 16:04 schrieb Alexander Nyssen >> >:
>>> 
>>> Hi Kevin,
>>> 
>>> attached please find an initial patch for 
>>> https://bugs.openjdk.java.net/browse/JDK-8161282 
>>> .
>>> 
>>> The patch is not as minimal as I had hoped, as the EmbeddedSceneInterface 
>>> had to be changed to differentiate between mouse and scroll events (while 
>>> up to now, scroll events are handled as mouse events), but for me this 
>>> seemed necessary to fix this issue properly. As a result, JFXPanel had to 
>>> be adjusted as well to comply to the changes in the EmbeddedSceneInterface, 
>>> while its behavior should not have changed.
>>> 
>>> As horizontal mouse events cannot be synthesized via Display.post(Event) 
>>> yet (an open issue for SWT), I did not add an automated test, but instead 
>>> added a manual one (FXCanvasMouseWheelEventsTest). Therefore, this patch 
>>> does not depend on the patch I provided earlier for JDK-8160325.
>>> 
>>> Best Regards,
>>> Alexander
>>> 
>>> 
>>> 
>>> 
>> 
>> =

Re: [PATCH] 8161282: FXCanvas does not forward horizontal mouse scroll events to the embedded scene

[Kevin Rushforth]

OK, I'll upload this revised version of the patch today.

-- Kevin


Alexander Nyssen wrote:

Hi Kevin,

please consider the following updated patch instead, which contains an 
additional null-check.


Regards
Alexander





Am 12.08.2016 um 16:04 schrieb Alexander Nyssen >:


Hi Kevin,

attached please find an initial patch for 
https://bugs.openjdk.java.net/browse/JDK-8161282.


The patch is not as minimal as I had hoped, as the 
EmbeddedSceneInterface had to be changed to differentiate between 
mouse and scroll events (while up to now, scroll events are handled 
as mouse events), but for me this seemed necessary to fix this issue 
properly. As a result, JFXPanel had to be adjusted as well to comply 
to the changes in the EmbeddedSceneInterface, while its behavior 
should not have changed.


As horizontal mouse events cannot be synthesized via 
Display.post(Event) yet (an open issue for SWT), I did not add an 
automated test, but instead added a manual one 
(FXCanvasMouseWheelEventsTest). Therefore, this patch does not depend 
on the patch I provided earlier for JDK-8160325.


Best Regards,
Alexander






=

Re: [PATCH] 8161282: FXCanvas does not forward horizontal mouse scroll events to the embedded scene

[Alexander Nyssen]

Hi Kevin,

please consider the following updated patch instead, which contains an 
additional null-check.

Regards
Alexander


> Am 12.08.2016 um 16:04 schrieb Alexander Nyssen :
> 
> Hi Kevin,
> 
> attached please find an initial patch for 
> https://bugs.openjdk.java.net/browse/JDK-8161282 
> .
> 
> The patch is not as minimal as I had hoped, as the EmbeddedSceneInterface had 
> to be changed to differentiate between mouse and scroll events (while up to 
> now, scroll events are handled as mouse events), but for me this seemed 
> necessary to fix this issue properly. As a result, JFXPanel had to be 
> adjusted as well to comply to the changes in the EmbeddedSceneInterface, 
> while its behavior should not have changed.
> 
> As horizontal mouse events cannot be synthesized via Display.post(Event) yet 
> (an open issue for SWT), I did not add an automated test, but instead added a 
> manual one (FXCanvasMouseWheelEventsTest). Therefore, this patch does not 
> depend on the patch I provided earlier for JDK-8160325.
> 
> Best Regards,
> Alexander
> 
> 
> 
>