On Tue, 11 Aug 2026 20:32:03 GMT, Alexander Zuev <[email protected]> wrote:
>> modules/javafx.graphics/src/main/java/javafx/scene/AccessibleAttribute.java >> line 814: >> >>> 812: * </ul> >>> 813: */ >>> 814: VISIBLE_ITEM_RANGE(int[].class), >> >> 1. this is a new attribute, so this PR needs as CSR, correct? >> 2. would it make more sense to use a more descriptive record instead of >> int[]? >> >> minor: the values in this enum are not sorted alphabetically - should they? >> do we care? > >> 1. this is a new attribute, so this PR needs as CSR, correct? > > I have no idea. If that would be a change in the public API i would > definitely say yes, for the attribute - since we add it and not changing the > meaning of the existing attribute - may be? Gray area for me honestly. > @kevinrushforth Any comments? > >> 2. would it make more sense to use a more descriptive record instead of >> int[]? > > That would require more processing on the native side so i would just leave > it as is. It is a technical attribute that is required to pass information > from the skin level to the native a11y helper - it is not supposed to be > available for any other purposes. > >> minor: the values in this enum are not sorted alphabetically - should they? >> do we care? > > Not really. Changing the existing code just to sort the enum - i would hate > to do so, i prefer to keep the history cleaner, it's an old code, technical > binding of the parameters to the native bound variables, nobody really looks > at it except the compiler. AccessibleAttribute is public API. The new attribute also needs `@since 28`. CSR is needed. (it's fine to keep it an `int[]`) ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/2217#discussion_r3761525540
