RFR: 8295248: JEditorPane HTML form with multi-selection broke data after resetting [v3]

Toshio Nakamura tnakamura at openjdk.org
Tue Oct 25 07:30:36 UTC 2022


On Mon, 24 Oct 2022 04:27:15 GMT, Phil Race <prr at openjdk.org> wrote:

>> Toshio Nakamura has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Fixed space
>
> Can you try again to describe the issue ? 
> The bug summary is misleading I'm sure. "Drawing" as in RENDERING is NOT broken.
> I think you are trying to say something about expectations for what item(s) are selected .. 
> Perhaps what is missing is a PROPER explanation of what you think reset and submit should do and so forth.
> And this looks like one of those "tiny" changes which are "completely harmless" but you have said NOTHING about what wider testing you've done on other cases. Fix one case != make things uniformly better ..

> @prrace has a good point here. I had to review the JBS and changes to really understand the issue. I guess the point is, why does model's removeIndexInterval not do what it should do here?
> 
> Visually clearSelection fixes the issue, but removeIndexInterval explicitly updates the states of the indexes in the method's body. clearSelection instead calls removeSelectionInterval which might do the same in a roundabout way.
> 
> I ran some tests on my end, and I still believe this fix is probably OK. It's just maybe better to explain more in depth why clearSelection is a suitable replacement for the removeIndexInterval block.

@DamonGuy 
Sorry I misunderstood at first.
OptionListModel.removeIndexInterval() and removeSelectionInterval() are completely different.
removeIndexInterval(i, i) *deletes* the indexed item, not unset.
removeSelectionInterval(i, i) *unsets* the indexed item.

To change to removeSelectionInterval() is also ok, but clearSelection() calls remoteSelectionInterval() more effectively. So, clearSelection() is suitable, I think.

-------------

PR: https://git.openjdk.org/jdk/pull/10685



More information about the client-libs-dev mailing list