8268087: Update documentation of the JPasswordField #4296

mrserb · 2021-06-02T01:14:39Z

Some useful documentation was added to the JPasswordField.

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
Change must be properly reviewed

Issue

JDK-8268087: Update documentation of the JPasswordField

Reviewers

Tejpal Rebari (@trebari - Committer) ⚠️ Review applies to ff208b7
Alexander Zvegintsev (@azvegint - Reviewer) ⚠️ Review applies to ff208b7
Phil Race (@prrace - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/4296/head:pull/4296
$ git checkout pull/4296

Update a local copy of the PR:
$ git checkout pull/4296
$ git pull https://git.openjdk.java.net/jdk pull/4296/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 4296

View PR using the GUI difftool:
$ git pr show -t 4296

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/4296.diff

bridgekeeper · 2021-06-02T01:15:16Z

👋 Welcome back serb! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2021-06-02T01:17:12Z

@mrserb The following label will be automatically applied to this pull request:

swing

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

mrserb · 2021-06-02T01:46:45Z

/csr

openjdk · 2021-06-02T01:47:36Z

@mrserb has indicated that a compatibility and specification (CSR) request is needed for this pull request.
@mrserb please create a CSR request for issue JDK-8268087. This pull request cannot be integrated until the CSR request is approved.

mlbridge · 2021-06-02T01:56:36Z

Webrevs

01: Full - Incremental (c0b4ef3d)
00: Full (ff208b78)

azvegint · 2021-06-02T13:08:08Z

src/java.desktop/share/classes/javax/swing/JPasswordField.java

+ * doesn't prevent the password from appearing in the system memory. For
+ * handling confidential information such as the password text, refer to the
+ * relevant section at
+ * <a href="https://www.oracle.com/java/technologies/javase/seccodeguide.html">


We might want to use the link with an anchor to the specific section, e.g.:
https://www.oracle.com/java/technologies/javase/seccodeguide.html#2-2

If we want to allow that the representation of what was typed is anything the UI decides then let's start from scratch,

The {@code JPasswordField} will not show the original characters that were typed, instead displaying alternative text or graphics.
However this doesn't prevent... [and then the rest is as originally proposed]

mrserb · 2021-06-02T23:49:44Z

The CSR is filed: https://bugs.openjdk.java.net/browse/JDK-8268149
Please take a look.

prrace · 2021-06-03T00:16:28Z

src/java.desktop/share/classes/javax/swing/JPasswordField.java

 * inherited method, <code>enableInputMethods(true)</code>.
 * <p>
+ * <strong>Warning:</strong> The text entered in {@code JPasswordField} displays
+ * something that was typed and does not show the original characters. This


"something that was typed" ? That reads to me as if it displays the characters you typed which isn't what you mean

the whole sentence is weird

Let's change this to
Although the text displayed in {@code JPasswordField} does not show the actual password characters typed, this does not prevent ...

The "something was typed" text is used already in the description of the JPasswordField, in the first sentence.

Not sure that the word "text" in the "Although the text displayed" is the correct one. It is not necessary a "text" but "something"

The previous text you refer to is presumably
"where the view indicates something was typed,"

that is fine. Your text is trying to say something else AND you added the word that which means you are referring to the specific thing ..

So you can't just copy and edit that.

And "text" in my version is fine.

So in case that wasn't clear, please change it to ny version

Another way of phrasing it would be

The text displayed in {@code JPasswordField} shows you that something was typed but does not show the original characters.

Note "that something" not "something that" and the rest of the rephrasing matters too.

I do not like the phrase "Although the text displayed in", the "text" is not displayed in the JPasswordField per se, please suggest something else.

"The text entered in {@code JPasswordField} displays that something was typed and does not show the original characters"
?

change "and" to "but" and that will be good enough

"The text entered in {@code JPasswordField} displays that something was typed but does not show the original characters"

mlbridge · 2021-06-03T01:47:12Z

Mailing list message from Alan Snyder on swing-dev:

I think this statement is over-constraining the UI behavior.

What the password field displays does not have to be text in the ordinary sense of the word. It could be a series of large dots. It is really up to the UI.

Also, the sentence is turned around. It should not start with the ?text entered?, which is irrelevant.

Also, a UI may allow the user to request that the entered text be displayed. That should not be ruled out.

How about:

For security reasons, JPasswordField does not display the entered text. (Some UIs may allow the user to request the display of the entered text.)

mlbridge · 2021-06-03T01:47:13Z

Mailing list message from Alan Snyder on swing-dev:

I think this statement is over-constraining the UI behavior.

What the password field displays does not have to be text in the ordinary sense of the word. It could be a series of large dots. It is really up to the UI.

Also, the sentence is turned around. It should not start with the ?text entered?, which is irrelevant.

Also, a UI may allow the user to request that the entered text be displayed. That should not be ruled out.

How about:

For security reasons, JPasswordField does not display the entered text. (Some UIs may allow the user to request the display of the entered text.)

prrace · 2021-06-03T05:57:23Z

Mailing list message from Alan Snyder on swing-dev:

I think this statement is over-constraining the UI behavior.

What the password field displays does not have to be text in the ordinary sense of the word. It could be a series of large dots. It is really up to the UI.

It is a text field. It displays unicode text. Nothing else. It can't display a graphic such as a dot unless it is a unicode
assigned code point. So if we want to re-define text across the platform to exclude some subset of unicode it will be
a questionable and difficult job and this informational comment isn't the place to start.

mrserb · 2021-06-03T08:10:52Z

Mailing list message from Alan Snyder on swing-dev:
How about:
For security reasons, JPasswordField does not display the entered text. (Some UIs may allow the user to request the display of the entered text.)

Your text is already described in the JPasswordField, this is for what this component was created. The current update is just a note that even if the entered text is not displayed to the user it is still in the heap and can be accessed. The security guide will provide some useful information about such cases.

It is a text field. It displays unicode text. Nothing else. It can't display a graphic such as a dot unless it is a unicode
assigned code point. So if we want to re-define text across the platform to exclude some subset of unicode it will be
a questionable and difficult job and this informational comment isn't the place to start.

In fact, the JPasswordField is not a text component, and UI "can use whatever graphic techniques it desires to represent the field". This is the reason why the initial text in this fix contained "The text entered in {@code JPasswordField} displays something that was typed". So yes it really may contain something that was typed, in opposite to "displays that something was typed".

mlbridge · 2021-06-03T13:57:12Z

Mailing list message from Alan Snyder on swing-dev:

On Jun 2, 2021, at 11:00 PM, Phil Race <prr at openjdk.java.net> wrote:

It is a text field. It displays unicode text. Nothing else. It can't display a graphic such as a dot unless it is a unicode
assigned code point. So if we want to re-define text across the platform to exclude some subset of unicode it will be
a questionable and difficult job and this informational comment isn't the place to start.

Needless to say, I am not suggesting anything of the sort. You seem to be nitpicking my words rather than trying to understand my suggestion.

The component UI has complete control over painting. It can paint anything it wants, not just text. Normally, that would be a terrible idea, but in the case of a password field, the goal is to not display the entered text, but give very limited feedback when the user types a character.

Alan

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/swing-dev/attachments/20210603/152d0f81/attachment.htm>

mlbridge · 2021-06-03T13:57:13Z

Mailing list message from Alan Snyder on swing-dev:

On Jun 2, 2021, at 11:00 PM, Phil Race <prr at openjdk.java.net> wrote:

It is a text field. It displays unicode text. Nothing else. It can't display a graphic such as a dot unless it is a unicode
assigned code point. So if we want to re-define text across the platform to exclude some subset of unicode it will be
a questionable and difficult job and this informational comment isn't the place to start.

Needless to say, I am not suggesting anything of the sort. You seem to be nitpicking my words rather than trying to understand my suggestion.

The component UI has complete control over painting. It can paint anything it wants, not just text. Normally, that would be a terrible idea, but in the case of a password field, the goal is to not display the entered text, but give very limited feedback when the user types a character.

Alan

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/swing-dev/attachments/20210603/152d0f81/attachment.htm>

mlbridge · 2021-06-03T14:08:12Z

Mailing list message from Alan Snyder on swing-dev:

On Jun 3, 2021, at 1:13 AM, Sergey Bylokhov <serb at openjdk.java.net> wrote:

This is the reason why the initial text in this fix contained "The text entered in {@code JPasswordField} displays something that was typed". So yes it really may contain something that was typed, in opposite to "displays that something was typed?.

I think we are in agreement on the intent.

However, that sentence makes no sense to me.

The text does not display something. The component displays something.

The phrase ?something that was typed? means nothing to me.

The similar phrase ?what was typed? makes sense, but the component does not display "what was typed", that is the whole point.

The original ?the view indicates something was typed? is appropriate, although it would read better as ?the view indicates that something was typed?.

If you need to say more, perhaps it should be identified as an implementation note.

Alan

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/swing-dev/attachments/20210603/fe65d91c/attachment.htm>

mlbridge · 2021-06-03T14:08:13Z

Mailing list message from Alan Snyder on swing-dev:

On Jun 3, 2021, at 1:13 AM, Sergey Bylokhov <serb at openjdk.java.net> wrote:

This is the reason why the initial text in this fix contained "The text entered in {@code JPasswordField} displays something that was typed". So yes it really may contain something that was typed, in opposite to "displays that something was typed?.

I think we are in agreement on the intent.

However, that sentence makes no sense to me.

The text does not display something. The component displays something.

The phrase ?something that was typed? means nothing to me.

The similar phrase ?what was typed? makes sense, but the component does not display "what was typed", that is the whole point.

The original ?the view indicates something was typed? is appropriate, although it would read better as ?the view indicates that something was typed?.

If you need to say more, perhaps it should be identified as an implementation note.

Alan

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/swing-dev/attachments/20210603/fe65d91c/attachment.htm>

mlbridge · 2021-06-03T19:05:18Z

Mailing list message from Alan Snyder on swing-dev:

LGTM

On Jun 3, 2021, at 7:41 AM, Phil Race <prr at openjdk.java.net> wrote:

On Wed, 2 Jun 2021 13:08:08 GMT, Alexander Zvegintsev <azvegint at openjdk.org> wrote:

Some useful documentation was added to the JPasswordField.

src/java.desktop/share/classes/javax/swing/JPasswordField.java line 68:

66: * handling confidential information such as the password text, refer to the
67: * relevant section at
68: * <a href="https://www.oracle.com/java/technologies/javase/seccodeguide.html">

We might want to use the link with an anchor to the specific section, e.g.:
https://www.oracle.com/java/technologies/javase/seccodeguide.html#2-2

If we want to allow that the representation of what was typed is anything the UI decides then let's start from scratch,

The {@code JPasswordField} will not show the original characters that were typed, instead displaying alternative text or graphics.
However this doesn't prevent... [and then the rest is as originally proposed]

mrserb · 2021-06-03T21:31:55Z

The patch and CSR are updated.

openjdk · 2021-06-04T20:35:46Z

@mrserb This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8268087: Update documentation of the JPasswordField

Reviewed-by: trebari, azvegint, prr

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 91 new commits pushed to the master branch:

48dc72b: 8268272: Remove JDK-8264874 changes because Graal was removed.
20b6312: 8268151: Vector API toShuffle optimization
64ec8b3: 8212155: Race condition when posting dynamic_code_generated event leads to JVM crash
cd0678f: 8199318: add idempotent copy operation for Map.Entry
b27599b: 8268222: javax/xml/jaxp/unittest/transform/Bug6216226Test.java failed, cannot delete file
59a539f: 8268129: LibraryLookup::ofDefault leaks symbols from loaded libraries
40c9e25: 8265444: Javadocs: jdk.jshell - small typo
069f180: 8268174: Move x86-specific stub declarations into stubRoutines_x86.hpp
3025f05: 8264305: Create implementation for native accessibility peer for Statusbar java role
e2d5ff9: 8268214: Use system zlib and disable dtrace when building linux-aarch64 at Oracle
... and 81 more: https://git.openjdk.java.net/jdk/compare/c06db45fa77c8a90518d6ff023de6c46b7c89997...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

mrserb · 2021-06-05T15:40:06Z

/integrate

openjdk · 2021-06-05T15:41:30Z

@mrserb Since your change was applied there have been 99 commits pushed to the master branch:

91f9adc: 8268139: CDS ArchiveBuilder may reference unloaded classes
36bff6f: 8066694: Strange code in JavacParser.java
6c838c5: 8266846: Add java.time.InstantSource
7f55dc1: 8179880: Refactor javax/security shell tests to plain java tests
7e41ca3: 8266957: SA has not followed JDK-8220587 and JDK-8224965
6ff978a: 8267204: Expose access to underlying streams in Reporter
76b54a1: 8263512: [macos_aarch64] issues with calling va_args functions from invoke_native
4e6748c: 8267687: ModXNode::Ideal optimization is better than Parse::do_irem
48dc72b: 8268272: Remove JDK-8264874 changes because Graal was removed.
20b6312: 8268151: Vector API toShuffle optimization
... and 89 more: https://git.openjdk.java.net/jdk/compare/c06db45fa77c8a90518d6ff023de6c46b7c89997...master

Your commit was automatically rebased without conflicts.

Pushed as commit b2e9eb9.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

Update JPasswordField.java

1b207c9

openjdk bot added the swing client-libs-dev@openjdk.org label Jun 2, 2021

Update JPasswordField.java

ff208b7

openjdk bot added the csr Pull request needs approved CSR before integration label Jun 2, 2021

mrserb marked this pull request as ready for review June 2, 2021 01:52

openjdk bot added the rfr Pull request is ready for review label Jun 2, 2021

trebari approved these changes Jun 2, 2021

View reviewed changes

azvegint approved these changes Jun 2, 2021

View reviewed changes

prrace reviewed Jun 3, 2021

View reviewed changes

Update JPasswordField.java

c0b4ef3

prrace approved these changes Jun 4, 2021

View reviewed changes

openjdk bot removed the csr Pull request needs approved CSR before integration label Jun 4, 2021

openjdk bot added the ready Pull request is ready to be integrated label Jun 4, 2021

openjdk bot closed this Jun 5, 2021

openjdk bot added integrated Pull request has been integrated and removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Jun 5, 2021

mrserb deleted the JDK-8268087 branch June 7, 2021 04:51

8268087: Update documentation of the JPasswordField #4296

8268087: Update documentation of the JPasswordField #4296

Uh oh!

Conversation

mrserb commented Jun 2, 2021 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Reviewing

Uh oh!

bridgekeeper bot commented Jun 2, 2021

Uh oh!

openjdk bot commented Jun 2, 2021

Uh oh!

mrserb commented Jun 2, 2021

Uh oh!

openjdk bot commented Jun 2, 2021

Uh oh!

mlbridge bot commented Jun 2, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

azvegint Jun 2, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mrserb commented Jun 2, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mlbridge bot commented Jun 3, 2021

Uh oh!

mlbridge bot commented Jun 3, 2021

Uh oh!

prrace commented Jun 3, 2021

Uh oh!

mrserb commented Jun 3, 2021

Uh oh!

mlbridge bot commented Jun 3, 2021

Uh oh!

mlbridge bot commented Jun 3, 2021

Uh oh!

mlbridge bot commented Jun 3, 2021

Uh oh!

mlbridge bot commented Jun 3, 2021

Uh oh!

mlbridge bot commented Jun 3, 2021

Uh oh!

mrserb commented Jun 3, 2021

Uh oh!

openjdk bot commented Jun 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mrserb commented Jun 5, 2021

Uh oh!

openjdk bot commented Jun 5, 2021

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

mrserb commented Jun 2, 2021 •

edited by openjdk bot

Loading

mlbridge bot commented Jun 2, 2021 •

edited

Loading

azvegint Jun 2, 2021 •

edited

Loading

openjdk bot commented Jun 4, 2021 •

edited

Loading