jdk 8288882: JFileChooser - empty (0 bytes) file is displayed as 1 KB

trafficstars

JFileChooser - empty file size issue fixed. For empty file, now the size 0 KB. Manual Test Case "FileSizeCheck.java" created.

Progress

[x] Change must be properly reviewed (1 review required, with at least 1 Reviewer)
[x] Change must not contain extraneous whitespace
[x] Commit message must refer to an issue

Issue

JDK-8288882: JFileChooser - empty (0 bytes) file is displayed as 1 KB

Reviewers

Alexey Ivanov (@aivanov-jdk - Reviewer) ⚠️ Review applies to a5ae29c1
Andy Goryachev (@andy-goryachev-oracle - no project role) ⚠️ Review applies to 084a82a7

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk pull/9327/head:pull/9327
$ git checkout pull/9327

Update a local copy of the PR:
$ git checkout pull/9327
$ git pull https://git.openjdk.org/jdk pull/9327/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 9327

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/9327.diff

Jun 29 '22 13:06 kumarabhi006

:wave: Welcome back kumarabhi006! 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.

Jun 29 '22 13:06 bridgekeeper[bot]

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

client

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.

Jun 29 '22 13:06 openjdk[bot]

Webrevs

Jun 29 '22 13:06 mlbridge[bot]

JFileChooser in Windows and Ubuntu is setting the filesize from two different files. In ubuntu, the method "public Component getTableCellRendererComponent(JTable table, Object value, boolean isSelected, boolean hasFocus, int row, int column)" in FilePane.java is formatting the length and return it back. In Windows, it uses ShellFolder to set FileSize column values and it is returned in terms of bytes in stead of KB (as shown by native tools). To handle filesize in windows, I will like to raise separate PR.

Jul 06 '22 12:07 kumarabhi006

In Windows, it uses ShellFolder to set FileSize column values and it is returned in terms of bytes in stead of KB (as shown by native tools).

This doesn't seem to be true for Windows, I see "0 KB" and "1 KB" for zero-byte and one-byte sized files correspondingly in Windows Explorer.

To handle filesize in windows, I will like to raise separate PR.

Are you saying you will submit a new bug to address the above?

Jul 07 '22 20:07 aivanov-jdk

In Windows, it uses ShellFolder to set FileSize column values and it is returned in terms of bytes in stead of KB (as shown by native tools).

This doesn't seem to be true for Windows, I see "0 KB" and "1 KB" for zero-byte and one-byte sized files correspondingly in Windows Explorer.

To handle filesize in windows, I will like to raise separate PR.

Are you saying you will submit a new bug to address the above?

For Windows, I would like to submit a new bug,

Jul 08 '22 07:07 kumarabhi006

File size shown in native is upto 1 decimal value. Now JFileChooser also shows filesize upto 1 decimal value similar to native. There is a change in conversion factor from 1024 to 1000 to show file size as per native. Filesize_Ubuntu FileSize_JFileChooser

Jul 20 '22 08:07 kumarabhi006

Created a directory with bug-id (8288882) name under "open/test/jdk/javax/swing/JFileChooser". Added manual test case file "FileSizeCheck.java" along with some dummy files to perform filesize checks.

Jul 21 '22 10:07 kumarabhi006

ZeroFileSizeCheck file has been deleted as it is moved to different directory and is of no longer use.

Jul 21 '22 10:07 kumarabhi006

GTK_LAF Nimbus_LAF

Attached the screenshot for GTK LAF and Nimbus LAF for JFileChooser. In GTK LAF, JFileChooser doesn't show the file size. In Nimbus, JFileChooser shows file sizes correctly.

Jul 22 '22 07:07 kumarabhi006

Declared a variable "baseFileSize to replace 1000.0 value. Method formatToDoubleValue has been moved inside class DetailsTableCellRenderer.

Jul 22 '22 07:07 kumarabhi006

@prrace @andy-goryachev-oracle There are few changes as per review comments, so request to please review them.

Jul 27 '22 14:07 kumarabhi006

Do not check in files - you can generate these at run time and then delete them

This also means you don't need the extra level of folder - which should be named something else than the bug id anyway

Deleted the previously checked in test files. Added the code to create and delete the test files at run time. FileSizeCheck manual test case file moved outside the folder and kept inside JFileChooser directory. So, there is no extra level of folder now.

Jul 28 '22 10:07 kumarabhi006

@andy-goryachev-oracle Checked in Oracle Linux and it shows file size similar to native.

Aug 02 '22 18:08 kumarabhi006

Attached the screenshot for filesize in native Oracle linux and JFileChooser.

Oracle_Linux JFileChooser_Oracle_Linux

Aug 04 '22 06:08 kumarabhi006

Does the size formatting respect the user's setting for the decimal symbol?

Aug 05 '22 19:08 aivanov-jdk

@aivanov-jdk I have updated the suggested changes. JFileChooser has been added as a component in frame.

Aug 09 '22 13:08 kumarabhi006

@prsadhuk I have made few changes in formatting the file size, using MessageFormat and NumberFormat to format the file size with 1 decimal place precision. Earlier JFileChooser show file size for "1.0" as "1", now it will show one decimal place precision. Empty files show as "0 KB".

1_Decimal_Precision

Aug 09 '22 13:08 kumarabhi006

@prsadhuk I have made few changes in formatting the file size, using MessageFormat and NumberFormat to format the file size with 1 decimal place precision. Earlier JFileChooser show file size for "1.0" as "1", now it will show one decimal place precision. Empty files show as "0 KB".

If JFileChooser always shows a decimal point for nearly all files, should it show 0.0 KB for zero-sized files?

I guess it would simplify your code as there'll be no reason to change number format.

Aug 09 '22 19:08 aivanov-jdk

@prsadhuk I have made few changes in formatting the file size, using MessageFormat and NumberFormat to format the file size with 1 decimal place precision. Earlier JFileChooser show file size for "1.0" as "1", now it will show one decimal place precision. Empty files show as "0 KB".

If JFileChooser always shows a decimal point for nearly all files, should it show 0.0 KB for zero-sized files?

I guess it would simplify your code as there'll be no reason to change number format.

@aivanov-jdk It will simplify the code but I checked in native file system and it shows "0 bytes" for zero-sized files. So to keep it similar to native, zero-sized files shown as "0 KB".

Aug 10 '22 04:08 kumarabhi006

@aivanov-jdk I have updated as per the suggested changes.

Aug 10 '22 05:08 kumarabhi006

@prsadhuk I have made few changes in formatting the file size, using MessageFormat and NumberFormat to format the file size with 1 decimal place precision. Earlier JFileChooser show file size for "1.0" as "1", now it will show one decimal place precision. Empty files show as "0 KB".

If JFileChooser always shows a decimal point for nearly all files, should it show 0.0 KB for zero-sized files? I guess it would simplify your code as there'll be no reason to change number format.

@aivanov-jdk It will simplify the code but I checked in native file system and it shows "0 bytes" for zero-sized files. So to keep it similar to native, zero-sized files shown as "0 KB".

Yet still it's shows 0 KB in Java; all other sizes are showed with one digit after the decimal point, thus only 0 KB has special handling. It's different from the native system, and I am for consistency in Java approach. If we decided, we show the size as #.# where there's always a digit after the decimal point, I'd rather see it always displayed. It would be easier to scan the sizes.

I wonder how the file size of 200 bytes is shown: 1 KB or 0.2 KB.

Aug 10 '22 13:08 aivanov-jdk

@prsadhuk I have made few changes in formatting the file size, using MessageFormat and NumberFormat to format the file size with 1 decimal place precision. Earlier JFileChooser show file size for "1.0" as "1", now it will show one decimal place precision. Empty files show as "0 KB".

If JFileChooser always shows a decimal point for nearly all files, should it show 0.0 KB for zero-sized files? I guess it would simplify your code as there'll be no reason to change number format.

@aivanov-jdk It will simplify the code but I checked in native file system and it shows "0 bytes" for zero-sized files. So to keep it similar to native, zero-sized files shown as "0 KB".

Yet still it's shows 0 KB in Java; all other sizes are showed with one digit after the decimal point, thus only 0 KB has special handling. It's different from the native system, and I am for consistency in Java approach. If we decided, we show the size as #.# where there's always a digit after the decimal point, I'd rather see it always displayed. It would be easier to scan the sizes.

I wonder how the file size of 200 bytes is shown: 1 KB or 0.2 KB.

As of now files having size >0 and <1000 bytes are shown as 1.0 KB.

Aug 10 '22 14:08 kumarabhi006

I wonder how the file size of 200 bytes is shown: 1 KB or 0.2 KB.

As of now files having size >0 and <1000 bytes are shown as 1.0 KB.

And this seems wrong and inconsistent, doesn't it?

If we display the file size with one decimal point precision, then it doesn't hold at all until the file size reaches 1 KB.

Whatever the decision we make, I'd like to have a test for 1 KB file and for 0.5 KB file to document how it works. It's the point of having different file sizes in the test, right?

I agree the file sizes less than 1 KB are somewhat uncommon, I'm okay with displaying them as 1.0 KB.

Aug 10 '22 14:08 aivanov-jdk

I wonder how the file size of 200 bytes is shown: 1 KB or 0.2 KB.

As of now files having size >0 and <1000 bytes are shown as 1.0 KB.

And this seems wrong and inconsistent, doesn't it?

If we display the file size with one decimal point precision, then it doesn't hold at all until the file size reaches 1 KB.

Whatever the decision we make, I'd like to have a test for 1 KB file and for 0.5 KB file to document how it works. It's the point of having different file sizes in the test, right?

I agree the file sizes less than 1 KB are somewhat uncommon, I'm okay with displaying them as 1.0 KB.

Yeah, you are right if we display the file size with one decimal point precision then it is inconsistent. I will add the test files for 1 KB and 0.5 KB.

Aug 10 '22 15:08 kumarabhi006

If we display the file size with one decimal point precision, then it doesn't hold at all until the file size reaches 1 KB.

Is your point ( pun intended) that it looks like we are displaying files to a decimal precision, but in fact are not ?

ie it is very odd to display 1.0kb for files of 100, 200, 300, etc bytes .. What is the reason for displaying them all as 1.0kb .. this review is so long I'm not sure I am following any more.

Could we get a summary in a few sentences of the entire current proposal and the justification for it ?

Aug 10 '22 17:08 prrace

If we display the file size with one decimal point precision, then it doesn't hold at all until the file size reaches 1 KB.

Is your point ( pun intended) that it looks like we are displaying files to a decimal precision, but in fact are not ?

ie it is very odd to display 1.0kb for files of 100, 200, 300, etc bytes .. What is the reason for displaying them all as 1.0kb .. this review is so long I'm not sure I am following any more.

Could we get a summary in a few sentences of the entire current proposal and the justification for it ?

There was an issue with plural forms if we show file sizes in terms of bytes. So, it has been discussed that we can show the file size in terms of "1 KB" for files having size >0 and <1000 bytes. To keep file size similar to native file system, JFileChooser displays file sizes with one decimal point precision.

Now, the issue is whether an empty file should be displayed as "0.0 KB" or "0 KB" ? As of now empty files are displayed as "0 KB".

Aug 10 '22 18:08 kumarabhi006

If we display the file size with one decimal point precision, then it doesn't hold at all until the file size reaches 1 KB.

Is your point ( pun intended) that it looks like we are displaying files to a decimal precision, but in fact are not ?

That's exactly my point.

Could we get a summary in a few sentences of the entire current proposal and the justification for it ?

I also asked for the summary. That time it was a bit different: https://github.com/openjdk/jdk/pull/9327#discussion_r939828467

There was an issue with plural forms if we show file sizes in terms of bytes. So, it has been discussed that we can show the file size in terms of "1 KB" for files having size >0 and <1000 bytes. To keep file size similar to native file system, JFileChooser displays file sizes with one decimal point precision.

Now, the issue is whether an empty file should be displayed as "0.0 KB" or "0 KB" ? As of now empty files are displayed as "0 KB".

The original issue is that we displayed a zero-sized file as 1 KB. It's stated in the bug subject and in the bug description with more details.

The initial fix was to display it as 0 bytes, which leads to localisation problems, especially if we take into account 1 byte.

Eventually, it was decided to display the size in KB. Thus zero-sized file is displayed as 0 KB, and one-byte-sized file is displayed as 1 KB. This is fine.

Later on, the decimal point was added to mimic native file navigation app in Ubuntu. Now for files larger than 1 KB, the size is shown up to one decimal point. Yet all the files below 1 KB are still displayed as 1 KB. I think it is confusing. If we care to display the size with one decimal point for larger files, why can't we display the size of smaller files with the same precision?

So, 0 bytes is 0.0 KB; > 0 and <= 100 bytes is 0.1 KB, and so on.

Alternatively, drop the decimal altogether.

Aug 10 '22 18:08 aivanov-jdk

To have all files 1->1000 bytes be 1.0kB and yet 1,100 -> 1,200 bytes be 1.1kB does seem anomalous from a precision perspective, since the major order of magnitude would seem to be more important to describe.

Since we already aren't going to say "bytes" for those small files, then just getting that precision with the decimal point seems better - as well as being consistent !

Aug 10 '22 19:08 prrace

@prrace @aivanov-jdk I have modified the existing logic to show decimal precision for smaller files. Added few test cases for files having size <1000 bytes.

Aug 11 '22 10:08 kumarabhi006

jdk jdk copied to clipboard

8288882: JFileChooser - empty (0 bytes) file is displayed as 1 KB

Progress

Issue

Reviewers

Reviewing

Webrevs

jdk
jdk copied to clipboard