Skip to content

6955128: Spec for javax.swing.plaf.basic.BasicTextUI.getVisibleEditorRect contains inappropriate wording #25850

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+3 −4
Open

6955128: Spec for javax.swing.plaf.basic.BasicTextUI.getVisibleEditorRect contains inappropriate wording #25850

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions src/java.desktop/share/classes/javax/swing/plaf/basic/BasicTextUI.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1029,10 +1029,9 @@ public Dimension getMaximumSize(JComponent c) {


/**
* Gets the allocation to give the root View. Due
* to an unfortunate set of historical events this
* method is inappropriately named. The Rectangle
* returned has nothing to do with visibility.
* Gets the allocation to give the root View.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Gets the allocation to give the root View.
* Gets the allocation for the root view.

* The Rectangle returned has nothing to do with visibility but
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* The Rectangle returned has nothing to do with visibility but
* The rectangle returned has nothing to do with visibility but

I suggest going lower case unless you specifically refer to the rectangle as the type in which case, it should be {@code Rectangle}.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"nothing to do with" -> "unrelated to"

But what I'd really like to see is an explanation of what the return from this method ACTUALLY means / is used for.
What does "the allocation" mean ? Too woolly for me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getVisibleEditorRect() return the bounds of the text editor and the return value is used to to set-size/paint the
javax.swing.text.View of this text editor component.
"the allocation" refers to what allocated area the view needs to paint, it seems to be a common phrase used in context of View class which says

"The view gets its allocation from the parent at paint time, so it must be prepared to redo layout if the allocated area is different from what it is prepared to deal with. "

In that regard, probably getAllocatedViewRect() would have been better choice of the method name

Copy link
Member

@aivanov-jdk aivanov-jdk Jun 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The term allocation is commonly used in javax.swing.text.View and its subclasses, for example, The view gets its allocation from the parent at paint time, and While a view may render into its entire allocation, typically a view does not. (Emphasis mine.)

The View class has a method getChildAllocation.

Essentially, the allocation for text view means the size and position of the view. In this case, the returned rectangle is used an allocation for the root view to perform painting.

jdk/src/java.desktop/share/classes/javax/swing/plaf/basic/BasicTextUI.java

Lines 767 to 771 in 0dce98b

// paint the view hierarchy
Rectangle alloc = getVisibleEditorRect();
if (alloc != null) {
rootView.paint(g, alloc);
}

I do not think this method should explain what the allocation means; the javadoc may point to the View class.

Copy link
Member

@aivanov-jdk aivanov-jdk Jun 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@prsadhuk @prrace Does this description work?

    /**
     * Gets the allocation for the root view.
     * The returned rectangle is unrelated to visibility,
     * it is passed as the {@code allocation} parameter to
     * the {@link View#paint(Graphics, Shape) View.paint} method.
     * The component must have a non-zero positive size for
     * this translation to be computed.
     *
     * @return the bounding box for the root view
     */
    protected Rectangle getVisibleEditorRect() {

This renders like this:

Gets the allocation for the root view. The returned rectangle is unrelated to visibility, it is passed as the allocation parameter to the View.paint method. The component must have a non-zero positive size for this translation to be computed.

We can also add @TejeshR13's suggestion: “The method name is preserved for backward compatibility.”

Copy link
Contributor Author

@prsadhuk prsadhuk Jun 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is passed to View.setSize also so just mentioning one may not be right..and I guess {@code allocation} is a no-op as it is not a class..

Maybe we can mention "The returned rectangle is unrelated to visibility, and is used as an allocation parameter in javax.swing.text.View class to set the size and paint the allocated View"

Copy link
Member

@aivanov-jdk aivanov-jdk Jun 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is passed to View.setSize also so just mentioning one may not be right…

Right. Both uses cases fall into the same category: the allocation controls the size of the root view.

The returned allocation rectangle is also used in handling mutations to the text model.

I'd rather leave it without any explanation why it's called allocation. Anyone who dealt with Swing text components and their implementation of the view classes is expected to roughly understand what allocation means. Searching for usages of getVisibleEditorRect reveals the usages.

{@code allocation} is a no-op as it is not a class..

Yes, it is an op: it refers to allocation the parameter of the paint method.

Maybe we can mention "The returned rectangle is unrelated to visibility, and is used as an allocation parameter in javax.swing.text.View class to set the size and paint the allocated View"

If we stick to explaining allocation, your suggestion makes sense, I edited it a bit: “The returned rectangle is unrelated to visibility, it is used to set the size of the root view.” This is what it means… without going too deep into the details.

We could add @see links for View.paint and/or View.setSize, the link to paint could be the most useful because it has a parameter called allocation.

* the method is thus named to preserve backward compatibility.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* the method is thus named to preserve backward compatibility.
* the method name is preserved for backward compatibility.

If method name is retained from past then this sentence would be right ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. The above rephrasing sounds clearer.

* The component must have a non-zero positive size for
* this translation to be computed.
*
Expand Down