Skip to content

Fix LT-22520: Crash when closing Find dialog in Grammar Sketch#882

Merged
johnml1135 merged 1 commit into
mainfrom
LT-22520
May 13, 2026
Merged

Fix LT-22520: Crash when closing Find dialog in Grammar Sketch#882
johnml1135 merged 1 commit into
mainfrom
LT-22520

Conversation

@jtmaxwell3
Copy link
Copy Markdown
Collaborator

@jtmaxwell3 jtmaxwell3 commented May 13, 2026

This fixes https://jira.sil.org/browse/LT-22520. If you try to find something before a sketch has been generated, you get an error. Ideally, we would suppress the Find menu item in this case, but I wasn't sure that I could get that right in all cases. So now if you click on the Find menu item, nothing happens. This isn't perfect, but at least it doesn't crash. This is also pretty uncommon.


This change is Reviewable

@github-actions
Copy link
Copy Markdown

NUnit Tests

    1 files  ±0      1 suites  ±0   11m 37s ⏱️ -10s
4 187 tests ±0  4 116 ✅ ±0  71 💤 ±0  0 ❌ ±0 
4 196 runs  ±0  4 125 ✅ ±0  71 💤 ±0  0 ❌ ±0 

Results for commit fe43043. ± Comparison against base commit 96b9c6d.

@johnml1135
Copy link
Copy Markdown
Contributor

I took a look at the code and have a few thoughts:

  • Instead of going off of the title, a query to make sure that the JavaScript function appears more robust

On:

var browserJsQuery = "cleanUpHighlights()";
executor.EvaluateScript(browserJsQuery);

var browserJsQuery =
    "(function() {" +
    "  try {" +
    "    if (typeof cleanUpHighlights !== 'function') return false;" +
    "    cleanUpHighlights();" +
    "    return true;" +
    "  } catch (e) {" +
    "    return false;" +
    "  }" +
    "})();";
var ran = executor.EvaluateScript(browserJsQuery).ToBoolean();

I did a code search and there could also be similar issues at these locations. I don't know if they are risks or not. we could always open up a JIRA issue to address later if need be:

2: GeneratedHtmlViewer.cs:1088
Current behavior: assumes scrollToStoredPosition() exists.
Robust behavior: if missing, return false.

3: GeneratedHtmlViewer.cs:1123
Current behavior: assumes scrollToStoredPosition(...) exists and uses the boolean result.
Robust behavior: if missing, return false.

4: GeneratedHtmlViewer.cs:1151
Current behavior: assumes findAndHighlightText(...) exists and uses the returned count.
Robust behavior: if missing, return 0.

5: TryAWordDlg.cs:387
Current behavior: assumes cleanUpHighlights() exists.
Robust behavior: if missing, return false and do nothing.

@johnml1135
Copy link
Copy Markdown
Contributor

I think the code addresses the core issue - I am just wondering which is the most robust way to handle it. We could prevent the Find UI from coming up (probably the best way), but that is a greater scope creep.

@jtmaxwell3
Copy link
Copy Markdown
Collaborator Author

This is low priority, since users rarely invoke Find before generating a sketch. This code is good enough to avoid a crash, and simple enough to verify that it won't introduce any new bugs.

@johnml1135
Copy link
Copy Markdown
Contributor

I agree - it's not as clean as I would like, but it fixes it. The comment is clear enough - the check is for before data is loaded.

@johnml1135 johnml1135 self-requested a review May 13, 2026 23:34
@johnml1135 johnml1135 merged commit e26f6ea into main May 13, 2026
6 checks passed
@johnml1135 johnml1135 deleted the LT-22520 branch May 13, 2026 23:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants