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

[Alexander Scherbatiy]

    The fix looks good for me.

    Sorry for the FlowView -> BoxView hierarchy mistake.

   Thanks,
   Alexandr.

On 9/10/2013 5:53 PM, dmitry markov wrote:
> You are right. I corrected the fix. The webrev is located at 
> http://cr.openjdk.java.net/~dmarkov/8024395/jdk8/webrev.02/
>
> Thanks,
> Dmitry
> On 10/09/2013 17:04, Alexander Scherbatiy wrote:
>> On 9/10/2013 4:40 PM, dmitry markov wrote:
>>> Please find answer below.
>>
>>    One more remark.
>>
>>   The original forwardUpdate method has final index0 assignment as: 
>> index0 = Math.max(index0, 0);
>>   The new calculateUpdateIndexes method has final line: 
>> firstUpdateIndex = Math.max(firstUpdateIndex - 1, 0);
>>   It seems that logic was to get 0 or unchanged  index0 but now it 
>> gets 0 or decremented firstUpdateIndex value.
>>
>>   Thanks,
>>   Alexandr.
>>
>>
>>>
>>> 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
>>>>>>
>>>>>
>>>>
>>>
>>
>