oppia: [BUG]: The character-counting logic in the hint editor does not correctly account for RTE components.

Describe the bug

In the hint editor of the exploration editor page, there is a check to ensure that the hint is at most 500 characters long. However, this check does not properly account for noninteractive RTE components like image and math components, which are overweighted because the character count of their HTML tags is used.

(Note that this also applies to the hint editor in the question suggestions modal in the contributor dashboard – see #18879. That needs to be fixed too.)

Steps To Reproduce

Create a new exploration.
Add a TextInput interaction to it.
Create a new hint with an image and a few sentences of text.
You will see that the hint character count reaches the limit too quickly.

Expected Behavior

For the purposes of character count, rich-text components in rich-text fields should instead be “valued” as follows:

Link / Concept Card Link: the length of the displayed text
Collapsible / Tabs: 1000 characters (these components are too “big” to put in hints)
All other tags (image, math, video, etc.): 0 characters

The RTE character count logic should also be extracted to the central service core/templates/services/html-length.service.ts, and fully tested.

Screenshots/Videos

Screenshot from 2023-06-26 16-08-49

What device are you using?

Desktop

Operating System

Linux

What browsers are you seeing the problem on?

Chrome

Browser version

No response

Additional context

No response

Tips for developers

Before tackling the bug, please use git bisect (see https://git-scm.com/docs/git-bisect) to investigate which PR caused it (you only need to go back as far as commit https://github.com/oppia/oppia/commits/9a334e9). If you find the PR, leave a comment on this issue pointing to it, or just say that it happened before commit 9a334e9 if you could reproduce it there. This will help us fix the issue by reverting the faulty PR.

Also, if this is your first issue, please make sure to follow https://github.com/oppia/oppia/wiki/Contributing-code-to-Oppia#choosing-a-good-first-issue and https://github.com/oppia/oppia/wiki/Contributing-code-to-Oppia#setting-things-up before claiming it. Thanks!

About this issue

Original URL
State: closed
Created a year ago
Comments: 31 (17 by maintainers)

Commits related to this issue

Fix #18505 : Corrected character-counting logic in the hint editor (#19518) * Corrected Logic for Hint Editor * Corrected Logic for Hint Editor * Corrected Logic for Hint Editor * Corrected Logic ... — committed to oppia/oppia by HardikGoyal2003 4 months ago
Fix #18505 : Corrected character-counting logic in the hint editor (#19518) * Corrected Logic for Hint Editor * Corrected Logic for Hint Editor * Corrected Logic for Hint Editor * Corrected Logic ... — committed to oppia/oppia by HardikGoyal2003 4 months ago
Fix #18505: Updated empty string error log (#19883) * Removed logging * Added string_type * Added Solution * Revert "Added string_type" This reverts commit d3c9bb699a74ca1e20f7fe8190b8b57950dbe1... — committed to oppia/oppia by HardikGoyal2003 4 months ago
Fix #18505: Updated empty string error log (#19883) * Removed logging * Added string_type * Added Solution * Revert "Added string_type" This reverts commit d3c9bb699a74ca1e20f7fe8190b8b57950dbe1... — committed to oppia/oppia by HardikGoyal2003 4 months ago
Fix #18505: Updated empty string error log (#19883) * Removed logging * Added string_type * Added Solution * Revert "Added string_type" This reverts commit d3c9bb699a74ca1e20f7fe8190b8b57950dbe1... — committed to oppia/oppia by HardikGoyal2003 4 months ago
Fix #18505 : Corrected character-counting logic in the hint editor (#19518) * Corrected Logic for Hint Editor * Corrected Logic for Hint Editor * Corrected Logic for Hint Editor * Corrected Logic ... — committed to oppia/oppia by HardikGoyal2003 4 months ago

Most upvoted comments

@eswalkerUmich Thanks, this looks better, I’m happy to assign you. Please feel free to open a PR and we can do the rest of the comments there.

Some more notes:

Yes, addHint should call the same methods to check validity. Thanks for looking at that as well. As usual, please make sure that the code is not duplicated – there should be one method for any piece of logic and different callers can call it as needed.
Please don’t use a countCharacters boolean to encode whether to count characters or words. Instead, one approach you can take is to define functions for countCharacters and countWords, and pass those functions in as the second argument so that you have something like calculateBaselineLength(sanitizedHtml, countingMethod). This is a more “even” way of representing the different counting strategies, and allows us to add other aggregation methods later if necessary, too.
For the non-text nodes I suggest that you instead have generic functions for getCountOfMathTags() and getAltTextsForImages() that both functions use, and keep the weighting logic in the computeHtmlLengthInX() functions. That’s because the decisions there are pretty specific to those functions and any changes should be made in the context of the overall function.

We can defer other things to the PR review. Thanks for taking a look at this and improving the code!

seanlip on Nov 30, 2023

@HardikGoyal2003 Per the guidance at https://github.com/oppia/oppia/wiki/Contributing-code-to-Oppia#choosing-a-good-first-issue, please provide an explanation of what your PR will do (with names of files you’re changing, what you plan to change in each file, etc.). If it looks good, we can assign you to this issue.

Please also follow the other instructions on that wiki page if you have not yet done so. Thanks!

Finally please see this comment for guidance on how to get started with this issue. If you can show a video with proof that the hint checks work reasonably now, and describe what changes you’ve made to address this, we can assign you the issue. Thanks!

seanlip on Oct 7, 2023