Problem/Motivation

As title, and as a part of #3122088: [Meta] Remove spelling errors from dictionary.txt and fix them

Proposed resolution

  1. Correcting wiget to widget
  2. escapeable -> escapable
  3. PHPunit -> PHPUnit gets punit removed form the directory

One typo in assets/vendor/jquery.ui/ui/widgets/button.js, remember running cd core && yarn && yarn build:jqueryui to regenerate assets/vendor/jquery.ui/ui/widgets/button-min.js

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#13 interdiff-11-13.txt273 bytesjungle
#13 3160124-13.patch4.15 KBjungle
#11 interdiff-7-11.txt11.86 KBjungle
#11 3160124-11.patch4.07 KBjungle
#7 interdiff-5-7.txt11 KBjameszhang023
#7 3160124-7.patch15.74 KBjameszhang023
#5 3160124-5.patch4.74 KBjameszhang023
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jungle created an issue. See original summary.

jungle’s picture

jungle
He/Him
Location Chongqing, China
CreditAttribution: jungle as a volunteer and at InterGreat Education Group commented
Title: Fix "wiget" typo in Core » Fix "wiget, escapeable,PHPunit" typos in Core
Issue summary: View changes

Adding two more found to here

escapeable -> escapable
PHPunit -> PHPUnit gets punit removed form the directory

jungle’s picture

jungle
He/Him
Location Chongqing, China
CreditAttribution: jungle as a volunteer and at InterGreat Education Group commented
Title: Fix "wiget, escapeable,PHPunit" typos in Core » Fix "wiget, escapeable, PHPunit" typos in Core

Adding one space after , to the title

jameszhang023’s picture

jameszhang023 CreditAttribution: jameszhang023 commented
Assigned: Unassigned » jameszhang023
Status: Active » Needs work

working on this

jameszhang023’s picture

jameszhang023 CreditAttribution: jameszhang023 commented
Assigned: jameszhang023 » Unassigned
Status: Needs work » Needs review
FileSize
3160124-5.patch4.74 KB

done, please review, thanks.

jungle’s picture

jungle
He/Him
Location Chongqing, China
CreditAttribution: jungle as a volunteer and at InterGreat Education Group commented
Status: Needs review » Needs work

Thanks, @jameszhang023! Looks like you forgot to run cd core && yarn && yarn build:jqueryui to regenerate assets/vendor/jquery.ui/ui/widgets/button-min.js.

jameszhang023’s picture

jameszhang023 CreditAttribution: jameszhang023 commented
Status: Needs work » Needs review
FileSize
3160124-7.patch15.74 KB
interdiff-5-7.txt11 KB

Apologize, thank you @jungle pointing out the problem that I forgot to run cd core && yarn && yarn build:jqueryui in #5. please review, thanks.

jungle’s picture

jungle
He/Him
Location Chongqing, China
CreditAttribution: jungle as a volunteer and at InterGreat Education Group commented

A wrong comment, sorry.
(deleted)

jungle’s picture

jungle
He/Him
Location Chongqing, China
CreditAttribution: jungle as a volunteer and at InterGreat Education Group commented
Status: Needs review » Reviewed & tested by the community 
$ cd core && yarn && yarn spellcheck:core
yarn run v1.22.4
$ cspell "**/*" "../composer/**/*" "../composer.json"
CSpell: Files checked: 14630, Issues found: 0 in 0 files
✨  Done in 239.72s.

2 CS violations, but JS relevant.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Status: Reviewed & tested by the community » Needs work 
+++ b/core/assets/vendor/jquery.ui/ui/widgets/button-min.js

+++ b/core/assets/vendor/jquery.ui/ui/widgets/button.js
@@ -181,7 +181,7 @@ $.widget( "ui.button", {
-			this._removeClass( this.icon, null, "ui-wiget-icon-block" );
+			this._removeClass( this.icon, null, "ui-widget-icon-block" );

This is in upstream vendor code. Yep it's a bug but we need to confirm it's fixed there and probably update here BUT jquery ui is no longer actively maintained and this will be deprecated at some point so I'm not sure that's worth it. Can we remove wiget from the list of spelling mistakes and not fix this? I think that'll work as long as we fix the other instance. In fact yeah we definitely should not be touching this in this issue.

jungle’s picture

jungle
He/Him
Location Chongqing, China
CreditAttribution: jungle as a volunteer and at InterGreat Education Group commented
Status: Needs work » Needs review
FileSize
3160124-11.patch4.07 KB
interdiff-7-11.txt11.86 KB

Thanks @alexpott for your input!

+++ b/core/modules/media_library/tests/src/FunctionalJavascript/ViewsUiIntegrationTest.php
@@ -55,7 +55,7 @@ public function testViewsAdmin() {
-    // Test the same routine but in the view for the table wiget.
+    // Test the same routine but in the view for the table widget.

Keep this one fixed on purpose, reverted wiget related change in dictionary and jQueryUI relevant. So keep title unchanged.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Status: Needs review » Needs work

We can still remove wiget from the dictionary

jungle’s picture

jungle
He/Him
Location Chongqing, China
CreditAttribution: jungle as a volunteer and at InterGreat Education Group commented
Status: Needs work » Needs review
FileSize
3160124-13.patch4.15 KB
interdiff-11-13.txt273 bytes 
--- a/core/assets/vendor/jquery.ui/ui/widgets/button-min.js
+++ b/core/assets/vendor/jquery.ui/ui/widgets/button-min.js

[.cspell.json]

    "ignorePaths": [
      ".cspell.json",
      "composer.lock",
      "assets/vendor/**",

Code of jQueryUI is ignored by cspell. Based on #11, wiget can be removed directly.

Thanks!

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented
Status: Needs review » Reviewed & tested by the community

Looking good.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 13: 3160124-13.patch, failed testing. View results

jungle’s picture

jungle
He/Him
Location Chongqing, China
CreditAttribution: jungle as a volunteer and at InterGreat Education Group commented
Status: Needs work » Reviewed & tested by the community

@longwave, thanks for RTBC-ing. A known random failure, re-queued.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Version: 9.1.x-dev » 8.9.x-dev
Status: Reviewed & tested by the community » Fixed

Committed 0345c33 and pushed to 9.1.x. Thanks!
Committed 275ac5a and pushed to 9.0.x - cherry-picked this to 8.9.x. Thanks!

Backport to 8.9.x and 9.0.x does not container core/misc/cspell/dictionary.txt

  • alexpott committed 0345c33 on 9.1.x 
    Issue #3160124 by jungle, jameszhang023, alexpott: Fix "wiget,...

  • alexpott committed 275ac5a on 9.0.x 
    Issue #3160124 by jungle, jameszhang023, alexpott: Fix "wiget,...

  • alexpott committed fd2f9a0 on 8.9.x 
    Issue #3160124 by jungle, jameszhang023, alexpott: Fix "wiget,...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.