RFR: 8291792: DefaultStyledDocument.setCharacterAttribute accepts negative length [v4]

[Phil Race]

On Wed, 24 Aug 2022 18:34:41 GMT, Tejesh R <tr at openjdk.org> wrote:

> > > > Although I've reviewed the CSR I'd prefer it not be finalized yet. I think it needs more changes. The spec is SILENT about what then happens if length is negative and SILENT about an out of range offset too. I think we should address these issues as well.
> > > > Also everywhere - in the CSR and the bug description and summary the text said the method is called setCharacterAttribute whereas it is setCharacterAttributes.
> > > 
> > > 
> > > For negative length, now it just returns the control without processing it whereas before it used to raise an exception in arrayCopy method. Upper bound for length should be the text length(which is to be modified/added) I feel, still even if it crosses overall text size that case is handled by limiting to text length. These are not mentioned in spec, so should we modify the spec by adding the range bounds for length......?
> > 
> > 
> > Yes I am saying we should mention all of this
> 
> Will this addition to spec be fine -
> 
> ```
>       * A write lock is held by this operation while changes
>       * are being made, and a DocumentEvent is sent to the listeners
>       * after the change has been successfully completed.
> +     *
> +     * <p>
> +     * The expected {@Code length} range is the length of the text
> +     * in which the attributes are set. If the length is <= 0, then no
> +     * attributes are set, the control returns. If the length is > the
> +     * length of text in which the attributes are to be set then the
> +     * extra length is trimmed.
> +     * </p>
> +     *
>       * <p>
>       * This method is thread safe, although most Swing methods
>       * are not. Please see
> ```

should be {@code .. } 
And the control doesn't return, the method does. I think you were trying to say control returns to the caller but that doesn't work here and the interaction with offset is cryptic .. assuming that's the reference to "trimmed".
Something that makes clear that the replace arg isn't used in such a case might help too.

I expect something like

     * {@code offset} and {@code length} define the range of the text over which the attributes are set.
     * If the length is <= 0, then no action is taken  and the method just returns.
     * If the offset is < 0 or >= the length of the text then no action is taken, and the method just returns
     *  Otherwise if {@code offset + length} will exceed the length of the  text then the affected range is truncated to that length
     * 
But YOU  need to verify this is all actually true ..

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

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