<Swing Dev> [8] Review request for 8024395: Improve fix for line break calculations

[dmitry markov]

Please find answer below.

Thanks,
Dmitry
On 10/09/2013 13:49, Alexander Scherbatiy wrote:
> On 9/10/2013 12:44 PM, dmitry markov wrote:
>> Hi Alexandr,
>>
>> Thanks for comments. I have updated the fix. New webrev is located at 
>> http://cr.openjdk.java.net/~dmarkov/8024395/jdk8/webrev.01/
>> Please find my answer below.
>      Thank you for your updates.
>
>   - The BoxView class repaints some parts in the forwardUpdate method 
> in case  if they are not valid. Is it also applicable for FlowView ?

Yes, I think, it is applicable for FlowView. It seems, this is out of 
scope of the fix, since it overrides forwardUpdate() method for 
LogicalView class (not FlowView). LogicalView does not inherit BoxView. 
It inherits View through CompositeView.
>   - Could you run JCK and regression tests related to JTextComponent 
> and javax.swing.text package and check Notepad, Stylepad and SwingSet2 
> demos that there are no regressions?

I have just checked Notepad, Stylepad and SwingSet2 demos look OK. Some 
regression tests failed but the fail set is the same as before and after 
fix.
>
>   Thanks,
>   Alexandr.
>
>
>>
>> Thanks,
>> Dmitry
>> On 09/09/2013 16:59, Alexander Scherbatiy wrote:
>>> On 9/6/2013 6:38 PM, dmitry markov wrote:
>>>> Hello,
>>>>
>>>> Could you review the fix:
>>>>   bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8024395
>>>>   webrev: http://cr.openjdk.java.net/~dmarkov/8024395/jdk8/webrev.00/
>>>>
>>>> The fix reverts the changes added for JDK-8014863 and override 
>>>> forwardUpdate() method for LogicalView class located at FlowView. 
>>>> In other words View.forwardUpdate() will go back to its original 
>>>> behavior and LogicalView.forwardUpdate() will send update event to 
>>>> all views followed by the changed place. This event will cause view 
>>>> to drop the cache and re-calculate its break points.
>>>
>>>     - The FlowView class extends BoxView and the BoxView calls 
>>> super.forwardUpdate(ec, e, a, f). Should the FlowView class also 
>>> have super call in the forwardUpdate method?
>> As you can see, the implementations of View.forwardUpdate() and 
>> LogicalView.forwardUpdate() are slightly different. I think, it is 
>> not necessary to do one thing twice. That is why 
>> LogicalView.forwardUpdate() does not have super call.
>>>     - It is usually not good to have duplicated codes in some 
>>> methods. Is it possible to unite the same parts of code of 
>>> forwardUpdatemethod from View and FlowView class to a method with 
>>> package access?
>> Done, see updated webrev
>>>     - It should be possible to use {@inheritDoc} tag for the method 
>>> doc.
>> Done, see updated webrev
>>>
>>>     Thanks,
>>>     Alexandr.
>>>
>>>>
>>>> Thanks,
>>>> Dmitry
>>>
>>
>