Skip to content

Exclude URLs from character count in Japanese#17970

Merged
marinakoleva merged 11 commits intorelease/18.0from
LINGO-1259-exclude-url-from-character-count
Jan 19, 2022
Merged

Exclude URLs from character count in Japanese#17970
marinakoleva merged 11 commits intorelease/18.0from
LINGO-1259-exclude-url-from-character-count

Conversation

@iolandasequino
Copy link
Copy Markdown
Contributor

@iolandasequino iolandasequino commented Jan 18, 2022

Context

Summary

This PR can be summarized in the following changelog entry:

  • Fixes a bug where the text length assessment would count URLs from videos loaded in the article in the total amount of the copy characters in Japanese.
  • Fixes a bug where Japanese full stops in a text would be counted as three characters instead of one.
  • [yoastseo] Removes spaces before and after Japanese full stops when sanitizing strings.
  • [yoastseo] Removes URLs from Japanese texts before computing text length in the countCharacters function.
  • [shopify-seo] Excludes URLs from the characters count in Japanese.

Relevant technical choices:

  • This fix was initially thought of for videos URLs embedded in HTML <figure> tags. The figure tag is used for all kinds of images and videos, and also captions are embedded in those. Since we include captions in our character count, removing the text embedded in those tags was not a viable solution. So as a result of this fix no URLs are taken into account in the characters count in Japanese.
  • At some point, we incorrectly add spaces before and after Japanese full periods, when there aren't any in the original text. These spaces are then counted when calculating total character count which results in inaccurate values. Ideally, we should investigate further to find out why and where this happens in the first place. But for the purpose of this bugfix, we added Japanese-specific regexes to the stripSpaces.js function where those spaces get removed when sanitizing the string.

Test instructions

Test instructions for the acceptance test before the PR gets merged

This PR can be acceptance tested by following these steps:

Testing the text length assessment

  • Build and activate Yoast SEO Free
  • Set your site to Japanese
  • Open/create a new post and add some Japanese text (you can use this generator)
  • Add a text of less than 600 characters to your text. You can count the amount of characters you added in your text with this character counter
  • Check how may characters are counted by the text length assessment (テキストの長さ)
  • Add a video by pasting a Youtube link to your article
  • Check that the number shown by the text length assessment has not changed
  • Add a video by loading a video from your laptop to your article
  • Check that the number shown by the text length assessment has not changed
  • Add a video by loading a video from the media library
  • Check that the number shown by the text length assessment has not changed
  • Add a URL to your article (e.g., www.yoast.com)
  • Check that the number shown by the text length assessment has not changed
  • Try pasting a few different URLs of different formats, e.g. https://en.wikipedia.org/wiki/Node.js, https://yoast.com/shopify-features/languages/, and make sure that the text length assessment result still doesn't change.

Testing the Video SEO video body assessment

  • When in Video SEO repository, require the Free branch created for this issue: composer require yoast/wordpress-seo:dev-LINGO-1259-exclude-url-from-character-count@dev
  • Build and activate Yoast SEO Free and Video SEO on your site
  • Make a post, add a video to it and save it
  • Add a Japanese text of 299 characters to the post (you can use this character counter)
    Confirm that the feedback of the Video body assessment returns an orange bullet with the following feedback: body のコピーは、検索エンジンが動画のトピックを理解するには短すぎます動画のコンテンツを説明するコンテンツをさらに追加してください。
  • Add a video URL from Youtube to your text
  • Make sure that the result for Video body assessment does not change.

Test that full stops are counted as only one character

  • Add the following sentences to your text: 吾輩は猫である。私は犬です。
  • Make sure that the text length reported by the text length assessment results increases by 14 characters.

Test instructions for QA when the code is in the RC

  • QA should use the same steps as above.

QA can test this PR by following these steps:

Impact check

This PR affects the following parts of the plugin, which may require extra testing:

UI changes

  • This PR changes the UI in the plugin. I have added the 'UI change' label to this PR.

Other environments

  • This PR also affects Shopify. I have added a changelog entry starting with [Shopify] and I have added test instructions for Shopify.

Documentation

  • I have written documentation for this change.

Quality assurance

  • I have tested this code to the best of my abilities
  • I have added unittests to verify the code works as intended
  • If any part of the code is behind a feature flag, my test instructions also cover cases where the feature flag is switched off.

Fixes https://yoast.atlassian.net/browse/LINGO-1259

@iolandasequino iolandasequino added the changelog: bugfix Needs to be included in the 'Bugfixes' category in the changelog label Jan 18, 2022
@iolandasequino iolandasequino added this to the 18.0 milestone Jan 18, 2022
@iolandasequino iolandasequino changed the base branch from trunk to release/18.0 January 18, 2022 16:42
@marinakoleva
Copy link
Copy Markdown
Contributor

CR done, all Testing instructions pass. However, I'm seeing the Video SEO specific feedback strings and the Video tab in English, instead of Japanese, so I'm not going to merge the issue yet.

@marinakoleva
Copy link
Copy Markdown
Contributor

Merge has been approved as no change regarding translation was done in this issue.

@marinakoleva marinakoleva merged commit b0baf9d into release/18.0 Jan 19, 2022
@marinakoleva marinakoleva deleted the LINGO-1259-exclude-url-from-character-count branch January 19, 2022 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog: bugfix Needs to be included in the 'Bugfixes' category in the changelog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants