<Swing Dev> RFR: 8263454: com.apple.laf.AquaFileChooserUI ignores the result of String.trim()

classic Classic list List threaded Threaded
13 messages Options
Reply | Threaded
Open this post in threaded view
|

<Swing Dev> RFR: 8263454: com.apple.laf.AquaFileChooserUI ignores the result of String.trim()

Alexander Zvegintsev-2
Looks like the original idea was to set `fallbacktext` on strings containing only spaces.

I decided to remove the `trim()` call to keep the same behavior and to allows to set such meaningless space only titles/tooltips(same as on other platforms, maybe someone want empty label for some reason).

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

Commit messages:
 - initial

Changes: https://git.openjdk.java.net/jdk/pull/3136/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=3136&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8263454
  Stats: 3 lines in 1 file changed: 0 ins; 2 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3136.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3136/head:pull/3136

PR: https://git.openjdk.java.net/jdk/pull/3136
Reply | Threaded
Open this post in threaded view
|

Re: <Swing Dev> RFR: 8263454: com.apple.laf.AquaFileChooserUI ignores the result of String.trim()

Sergey Bylokhov-2
On Mon, 22 Mar 2021 22:19:18 GMT, Alexander Zvegintsev <[hidden email]> wrote:

> Looks like the original idea was to set `fallbacktext` on strings containing only spaces.
>
> I decided to remove the `trim()` call to keep the same behavior and to allows to set such meaningless space only titles/tooltips(same as on other platforms, maybe someone want empty label for some reason).

Marked as reviewed by serb (Reviewer).

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

PR: https://git.openjdk.java.net/jdk/pull/3136
Reply | Threaded
Open this post in threaded view
|

Re: <Swing Dev> RFR: 8263454: com.apple.laf.AquaFileChooserUI ignores the result of String.trim()

Pankaj Bansal-2
In reply to this post by Alexander Zvegintsev-2
On Mon, 22 Mar 2021 22:19:18 GMT, Alexander Zvegintsev <[hidden email]> wrote:

> Looks like the original idea was to set `fallbacktext` on strings containing only spaces.
>
> I decided to remove the `trim()` call to keep the same behavior and to allows to set such meaningless space only titles/tooltips(same as on other platforms, maybe someone want empty label for some reason).

Marked as reviewed by pbansal (Reviewer).

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

PR: https://git.openjdk.java.net/jdk/pull/3136
Reply | Threaded
Open this post in threaded view
|

Re: <Swing Dev> RFR: 8263454: com.apple.laf.AquaFileChooserUI ignores the result of String.trim()

Prasanta Sadhukhan-2
In reply to this post by Alexander Zvegintsev-2
On Mon, 22 Mar 2021 22:19:18 GMT, Alexander Zvegintsev <[hidden email]> wrote:

> Looks like the original idea was to set `fallbacktext` on strings containing only spaces.
>
> I decided to remove the `trim()` call to keep the same behavior and to allows to set such meaningless space only titles/tooltips(same as on other platforms, maybe someone want empty label for some reason).

src/java.desktop/macosx/classes/com/apple/laf/AquaFileChooserUI.java line 2059:

> 2057:             final String tooltipText = fc.getApproveButtonToolTipText();
> 2058:             if (tooltipText != null) {
> 2059:                 if (!tooltipText.isEmpty()) return tooltipText;

In other L&F like Metal, Basic even empty text is allowed.. there's no check for isEmpty()....Should we do the same here too?

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

PR: https://git.openjdk.java.net/jdk/pull/3136
Reply | Threaded
Open this post in threaded view
|

Re: <Swing Dev> RFR: 8263454: com.apple.laf.AquaFileChooserUI ignores the result of String.trim()

Alexander Zuev-3
In reply to this post by Alexander Zvegintsev-2
On Mon, 22 Mar 2021 22:19:18 GMT, Alexander Zvegintsev <[hidden email]> wrote:

> Looks like the original idea was to set `fallbacktext` on strings containing only spaces.
>
> I decided to remove the `trim()` call to keep the same behavior and to allows to set such meaningless space only titles/tooltips(same as on other platforms, maybe someone want empty label for some reason).

Marked as reviewed by kizune (Reviewer).

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

PR: https://git.openjdk.java.net/jdk/pull/3136
Reply | Threaded
Open this post in threaded view
|

Re: <Swing Dev> RFR: 8263454: com.apple.laf.AquaFileChooserUI ignores the result of String.trim() [v2]

Alexander Zvegintsev-2
In reply to this post by Alexander Zvegintsev-2
> Looks like the original idea was to set `fallbacktext` on strings containing only spaces.
>
> I decided to remove the `trim()` call to keep the same behavior and to allows to set such meaningless space only titles/tooltips(same as on other platforms, maybe someone want empty label for some reason).

Alexander Zvegintsev has updated the pull request incrementally with one additional commit since the last revision:

  allow empty text

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

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3136/files
  - new: https://git.openjdk.java.net/jdk/pull/3136/files/06c5322d..9e4f2a35

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3136&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3136&range=00-01

  Stats: 8 lines in 1 file changed: 0 ins; 2 del; 6 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3136.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3136/head:pull/3136

PR: https://git.openjdk.java.net/jdk/pull/3136
Reply | Threaded
Open this post in threaded view
|

Re: <Swing Dev> RFR: 8263454: com.apple.laf.AquaFileChooserUI ignores the result of String.trim() [v2]

Alexander Zvegintsev-2
In reply to this post by Prasanta Sadhukhan-2
On Tue, 23 Mar 2021 05:00:45 GMT, Prasanta Sadhukhan <[hidden email]> wrote:

>> Alexander Zvegintsev has updated the pull request incrementally with one additional commit since the last revision:
>>
>>   allow empty text
>
> src/java.desktop/macosx/classes/com/apple/laf/AquaFileChooserUI.java line 2059:
>
>> 2057:             final String tooltipText = fc.getApproveButtonToolTipText();
>> 2058:             if (tooltipText != null) {
>> 2059:                 if (!tooltipText.isEmpty()) return tooltipText;
>
> In other L&F like Metal, Basic even empty text is allowed.. there's no check for isEmpty()....Should we do the same here too?

Yes, I think it would be better to allow to set empty text to conform with other LaF behavior.

Also `JFileChooser.getApproveButtonText()` and `JFileChooser.getApproveButtonToolTipText()` will now return correct values for empty strings.
e.g. before the fix if we call `setApproveButtonText("")` subsequent call to `getApproveButtonText()` will return passed string, but in fact fallback string is used.

I've also labeled the issue with `noreg-hard`.

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

PR: https://git.openjdk.java.net/jdk/pull/3136
Reply | Threaded
Open this post in threaded view
|

Re: <Swing Dev> RFR: 8263454: com.apple.laf.AquaFileChooserUI ignores the result of String.trim() [v2]

Sergey Bylokhov-2
On Tue, 23 Mar 2021 15:33:43 GMT, Alexander Zvegintsev <[hidden email]> wrote:

>> src/java.desktop/macosx/classes/com/apple/laf/AquaFileChooserUI.java line 2059:
>>
>>> 2057:             final String tooltipText = fc.getApproveButtonToolTipText();
>>> 2058:             if (tooltipText != null) {
>>> 2059:                 if (!tooltipText.isEmpty()) return tooltipText;
>>
>> In other L&F like Metal, Basic even empty text is allowed.. there's no check for isEmpty()....Should we do the same here too?
>
> Yes, I think it would be better to allow to set empty text to conform with other LaF behavior.
>
> Also `JFileChooser.getApproveButtonText()` and `JFileChooser.getApproveButtonToolTipText()` will now return correct values for empty strings.
> e.g. before the fix if we call `setApproveButtonText("")` subsequent call to `getApproveButtonText()` will return passed string, but in fact fallback string is used.
>
> I've also labeled the issue with `noreg-hard`.

Is it possible to set an empty label in the native file chooser on macOS?

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

PR: https://git.openjdk.java.net/jdk/pull/3136
Reply | Threaded
Open this post in threaded view
|

Re: <Swing Dev> RFR: 8263454: com.apple.laf.AquaFileChooserUI ignores the result of String.trim() [v2]

Tejpal Rebari-2
In reply to this post by Alexander Zvegintsev-2
On Tue, 23 Mar 2021 14:55:58 GMT, Alexander Zvegintsev <[hidden email]> wrote:

>> Looks like the original idea was to set `fallbacktext` on strings containing only spaces.
>>
>> I decided to remove the `trim()` call to keep the same behavior and to allows to set such meaningless space only titles/tooltips(same as on other platforms, maybe someone want empty label for some reason).
>
> Alexander Zvegintsev has updated the pull request incrementally with one additional commit since the last revision:
>
>   allow empty text

Marked as reviewed by trebari (Committer).

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

PR: https://git.openjdk.java.net/jdk/pull/3136
Reply | Threaded
Open this post in threaded view
|

Re: <Swing Dev> RFR: 8263454: com.apple.laf.AquaFileChooserUI ignores the result of String.trim() [v2]

Alexander Zvegintsev-2
In reply to this post by Sergey Bylokhov-2
On Tue, 23 Mar 2021 22:13:21 GMT, Sergey Bylokhov <[hidden email]> wrote:

>> Yes, I think it would be better to allow to set empty text to conform with other LaF behavior.
>>
>> Also `JFileChooser.getApproveButtonText()` and `JFileChooser.getApproveButtonToolTipText()` will now return correct values for empty strings.
>> e.g. before the fix if we call `setApproveButtonText("")` subsequent call to `getApproveButtonText()` will return passed string, but in fact fallback string is used.
>>
>> I've also labeled the issue with `noreg-hard`.
>
> Is it possible to set an empty label in the native file chooser on macOS?

Yes, and it looks the same as the one with `" "` label.
<img width="662" alt="image" src="https://user-images.githubusercontent.com/77687766/112301125-ea140e80-8c99-11eb-8588-b1730d43124f.png">

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

PR: https://git.openjdk.java.net/jdk/pull/3136
Reply | Threaded
Open this post in threaded view
|

Re: <Swing Dev> RFR: 8263454: com.apple.laf.AquaFileChooserUI ignores the result of String.trim() [v2]

Prasanta Sadhukhan-2
In reply to this post by Alexander Zvegintsev-2
On Tue, 23 Mar 2021 14:55:58 GMT, Alexander Zvegintsev <[hidden email]> wrote:

>> Looks like the original idea was to set `fallbacktext` on strings containing only spaces.
>>
>> I decided to remove the `trim()` call to keep the same behavior and to allows to set such meaningless space only titles/tooltips(same as on other platforms, maybe someone want empty label for some reason).
>
> Alexander Zvegintsev has updated the pull request incrementally with one additional commit since the last revision:
>
>   allow empty text

Marked as reviewed by psadhukhan (Reviewer).

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

PR: https://git.openjdk.java.net/jdk/pull/3136
Reply | Threaded
Open this post in threaded view
|

Re: <Swing Dev> RFR: 8263454: com.apple.laf.AquaFileChooserUI ignores the result of String.trim() [v2]

Sergey Bylokhov-2
In reply to this post by Alexander Zvegintsev-2
On Tue, 23 Mar 2021 14:55:58 GMT, Alexander Zvegintsev <[hidden email]> wrote:

>> Looks like the original idea was to set `fallbacktext` on strings containing only spaces.
>>
>> I decided to remove the `trim()` call to keep the same behavior and to allows to set such meaningless space only titles/tooltips(same as on other platforms, maybe someone want empty label for some reason).
>
> Alexander Zvegintsev has updated the pull request incrementally with one additional commit since the last revision:
>
>   allow empty text

Marked as reviewed by serb (Reviewer).

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

PR: https://git.openjdk.java.net/jdk/pull/3136
Reply | Threaded
Open this post in threaded view
|

<Swing Dev> Integrated: 8263454: com.apple.laf.AquaFileChooserUI ignores the result of String.trim()

Alexander Zvegintsev-2
In reply to this post by Alexander Zvegintsev-2
On Mon, 22 Mar 2021 22:19:18 GMT, Alexander Zvegintsev <[hidden email]> wrote:

> Looks like the original idea was to set `fallbacktext` on strings containing only spaces.
>
> I decided to remove the `trim()` call to keep the same behavior and to allows to set such meaningless space only titles/tooltips(same as on other platforms, maybe someone want empty label for some reason).

This pull request has now been integrated.

Changeset: c037e1ed
Author:    Alexander Zvegintsev <[hidden email]>
URL:       https://git.openjdk.java.net/jdk/commit/c037e1ed
Stats:     11 lines in 1 file changed: 0 ins; 4 del; 7 mod

8263454: com.apple.laf.AquaFileChooserUI ignores the result of String.trim()

Reviewed-by: serb, pbansal, kizune, trebari, psadhukhan

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

PR: https://git.openjdk.java.net/jdk/pull/3136