Skip to content
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.

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

Fix: tabs without file objects had null title in tab switcher #8213

Merged
merged 1 commit into from
Feb 6, 2025

Conversation

mbien
Copy link
Member

@mbien mbien commented Jan 30, 2025

image

targets delivery

@mbien mbien added Editor UI User Interface ci:dev-build [ci] produce a dev-build zip artifact (7 days expiration, see link on workflow summary page) labels Jan 30, 2025
@mbien mbien added this to the NB25 milestone Jan 30, 2025
Copy link
Collaborator

@troizet troizet left a comment

Choose a reason for hiding this comment

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

Thank you very much!

@mbien
Copy link
Member Author

mbien commented Feb 3, 2025

found a second issue unfortunately the file2file diff title is broken, trying to fix it

@mbien mbien marked this pull request as draft February 3, 2025 16:31
@mbien mbien force-pushed the fix-null-tab-title branch from d8cfe40 to 2c8323c Compare February 3, 2025 17:32
@mbien mbien requested review from troizet and eirikbakke February 3, 2025 17:32
@mbien mbien marked this pull request as ready for review February 3, 2025 17:33
@mbien
Copy link
Member Author

mbien commented Feb 3, 2025

fix is to escape the original tab title if:

  • the added prefix uses html
  • and the original tab title did not use html

didn't want to add a dependency to editor utils to get to StringEscapeUtils so I use the XML util which is essentially the same thing plus bonus exception.

This reminded me that we should probably put https://github.com/OWASP/owasp-java-encoder/ somewhere.

@@ -123,8 +125,18 @@ private static String merge( String prefix, String baseText ) {
res.append( prefix );
res.append( baseText.substring( 6 ) );
} else {
res.append( prefix );
res.append( baseText );
if (prefix.startsWith("<font")) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

//NOI18N ?

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

 - example: repo history with parent folder decorator enabled
 - or diff between two files
 - regression since ad776db
@mbien mbien force-pushed the fix-null-tab-title branch from 2c8323c to d9d269c Compare February 4, 2025 16:15
@mbien
Copy link
Member Author

mbien commented Feb 4, 2025

@neilcsmith-net @eirikbakke pinging for review.

IMO: this should not slip through rc2, we should either merge this or merge a revert of #7930. cc @ebarboni

@eirikbakke
Copy link
Contributor

eirikbakke commented Feb 4, 2025

Adjustments look fine. The handling of Swing HTML formatting is a bit ugly in this class, but bugs here are purely cosmetic in nature. (Swing HTML parsers, or the simplified parsers in NetBeans, do not execute Javascript or such.)

@mbien
Copy link
Member Author

mbien commented Feb 4, 2025

yeah, what makes it ugly is that it has to support all combinations of html text and plain text for both tab title and prefix. There are many places within NB which have to detect if a string is html or plain.

@eirikbakke
Copy link
Contributor

eirikbakke commented Feb 4, 2025

In my own NetBeans app I have a dedicated SwingString class for passing, concatenating, and escaping strings that may or may not contain Swing HTML formatting. That way the type checker takes care of tracking and documenting where HTML formatting may be used. Would probably be way too much work to adapt that in NetBeans, though.

@ebarboni
Copy link
Contributor

ebarboni commented Feb 5, 2025

LGTM

@ebarboni ebarboni merged commit 863d78c into apache:delivery Feb 6, 2025
31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci:dev-build [ci] produce a dev-build zip artifact (7 days expiration, see link on workflow summary page) Editor UI User Interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants