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 potential null access in TextEdit #90274

Merged
merged 1 commit into from
Jul 17, 2024

Conversation

AThousandShips
Copy link
Member

The highlighter would be accessed when setting_text == true regardless of it being valid

@AThousandShips AThousandShips added bug crash topic:gui cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release labels Apr 5, 2024
@AThousandShips AThousandShips added this to the 4.3 milestone Apr 5, 2024
@AThousandShips AThousandShips requested a review from a team as a code owner April 5, 2024 16:38
@@ -7780,7 +7780,7 @@ void TextEdit::_update_gutter_width() {

/* Syntax highlighting. */
Dictionary TextEdit::_get_line_syntax_highlighting(int p_line) {
return syntax_highlighter.is_null() && !setting_text ? Dictionary() : syntax_highlighter->get_line_syntax_highlighting(p_line);
return syntax_highlighter.is_null() || setting_text ? Dictionary() : syntax_highlighter->get_line_syntax_highlighting(p_line);
Copy link
Member Author

@AThousandShips AThousandShips Apr 5, 2024

Choose a reason for hiding this comment

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

I swapped it to setting_text as making it syntax_highlighter.is_null() || !setting_text made syntax highlighting go away in the editor, unsure why this condition is here but this way it didn't break the editor, if it's unnecessary I can remove that part

Copy link
Member

Choose a reason for hiding this comment

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

I would add parentheses because it's not clear to me what's the precedence between || and ?.

@AThousandShips AThousandShips modified the milestones: 4.3, 4.4 Jun 28, 2024
@AThousandShips AThousandShips added the cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release label Jun 28, 2024
@akien-mga akien-mga modified the milestones: 4.4, 4.3 Jul 17, 2024
@akien-mga akien-mga removed the cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release label Jul 17, 2024
@akien-mga akien-mga merged commit 82b0186 into godotengine:master Jul 17, 2024
18 checks passed
@akien-mga
Copy link
Member

Thanks!

@AThousandShips AThousandShips deleted the syntax_fix branch July 17, 2024 14:04
@AThousandShips
Copy link
Member Author

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release crash topic:gui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants