JDK-8142439 : Ensemble8 media player slider issues
Type:Bug
Component:javafx
Sub-Component:samples
Affected Version:9
Priority:P4
Status:Resolved
Resolution:Fixed
Submitted:2015-11-10
Updated:2020-01-31
Resolved:2016-10-20
The Version table provides details related to the release that this issue/RFE will be addressed.
Unresolved : Release in which this issue/RFE will be addressed. Resolved: Release in which this issue/RFE has been resolved. Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.
There needed to be a follow-on bug to 8134354 that addressed the advanced media sample and other nits that were found in a post-commit code review.
Comments
Webrev for 8u backport:
http://cr.openjdk.java.net/~kcr/Ensemble8/separate/03-8142439/webrev/
24-10-2016
This is part of an aggregate backport request to port Ensemble8 fixes from FX 9 to FX 8u. See JDK-8168611 for details and review.
24-10-2016
This was resolved almost a year ago, but the JBS bug was never updated.
http://hg.openjdk.java.net/openjfx/9-dev/rt/rev/3962c15cd0dc
changeset: 9376:3962c15cd0dc
user: Morris Meyer <morris.meyer@oracle.com>
date: Wed Nov 11 17:14:27 2015 -0500
summary: 8142439: Ensemble8 media player slider issues
20-10-2016
Morris,
I don't want you to spend any more time on these minor issues since the sample works okay, but still here's my thoughts:
I think that isValueChanging is meant to be used for sliders which should take action only after the user releases the thumb but still track the value change.
So since we are reacting to value change immediately I don't think there is any need to use this call.
Actually, updateValues is called anyway after the call of mp.seek() through the mp.currentTimeProperty() listener.
And it will call setValue on timeSlider again, duh!
BTW, there is an empty volumeSlider InvalidationListener at the line 305.
Anyway, sine the sample works correctly, that's a +1
11-11-2015
That is the correct assessment of the changes.
10-11-2015
It appears that the only substantive changes are:
1) Switched to using a change listener rather than an invalidation listener for timeSlider.valueProperty()
2) Added an else block in the above listener looking for a large enough change in value to seek to the appropriate place without calling updateValues().
3) Removes a couple redundant if tests where the body of the "if" and "else" blocks were identical
Did I miss anything?
It looks fine to me, so I'll defer to Vadim in case he has something else in mind.