WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#25947 closed defect (bug) (fixed)

jshint shouldn't throw errors: wp-admin/js/editor.js

Reported by: dougwollison Owned by: azaozz
Milestone: 3.8 Priority: normal
Severity: normal Version:
Component: Build/Test Tools Keywords: has-patch
Focuses: Cc:

Description

Pretty much only spacing fixes.

Attachments (5)

25947.diff (36.0 KB) - added by dougwollison 6 years ago.
initial patch; fixed spacing
25947.2.diff (15.8 KB) - added by dougwollison 6 years ago.
Attempt #2; fixed spacing, braces, equals, declared globals (not sure if I used the proper method though).
25947.3.diff (15.6 KB) - added by dougwollison 6 years ago.
Corrected handling of globals. 1 error left; 'switchEditors' is defined but never used
25947.4.diff (15.6 KB) - added by dougwollison 6 years ago.
Revised declaration of globals, restored case of tinyMCE
25947.5.diff (15.6 KB) - added by dougwollison 6 years ago.
Updated to include proper lowercasing of just tinymce

Download all attachments as: .zip

Change History (21)

@dougwollison
6 years ago

initial patch; fixed spacing

#1 @dougwollison
6 years ago

  • Keywords has-patch added

#2 @jorbin
6 years ago

  • Summary changed from jshint shouldn't throw errors: to jshint shouldn't throw errors: wp-admin/js/editor.js

Please remember to include the filename

#3 @dougwollison
6 years ago

Whoops, forgot while trying to get this submitted from the proper account.

#4 @nacin
6 years ago

Hi dougwollison, thanks for the patch.

It looks like some query.php documentation snuck into this one. (Also: wow, nice work!)

I tested out the patch and the spacing looks great. But it doesn't look like anything was done to fix JSHint errors. :-)

Here's the ones I found when running it:

Running "jshint:core" (jshint) task
Linting src/wp-admin/js/editor.js ...ERROR
[L197:C109] W109: Strings must use singlequote.
    pee = pee.replace( new RegExp( '<p>\\s*(</?(?:' + blocklist + ')(?: [^>]*)?>)\\s*</p>', 'gi' ), "$1" ); 
Linting src/wp-admin/js/editor.js ...ERROR
[L201:C101] W109: Strings must use singlequote.
    pee = pee.replace( new RegExp( '<p>\\s*(</?(?:' + blocklist + ')(?: [^>]*)?>)', 'gi' ), "$1" ); 
Linting src/wp-admin/js/editor.js ...ERROR
[L202:C102] W109: Strings must use singlequote.
    pee = pee.replace( new RegExp( '(</?(?:' + blocklist + ')(?: [^>]*)?>)\\s*</p>', 'gi' ), "$1" ); 
Linting src/wp-admin/js/editor.js ...ERROR
[L204:C98] W109: Strings must use singlequote.
    pee = pee.replace( new RegExp( '(</?(?:' + blocklist + ')[^>]*>)\\s*<br />', 'gi' ), "$1" ); 
Linting src/wp-admin/js/editor.js ...ERROR
[L16:C28] W117: 'tinyMCE' is not defined.
    var t = this, ed = tinyMCE.get( id ), wrap_id, txtarea_el, dom = tinymce.DOM;
Linting src/wp-admin/js/editor.js ...ERROR
[L16:C74] W117: 'tinymce' is not defined.
    var t = this, ed = tinyMCE.get( id ), wrap_id, txtarea_el, dom = tinymce.DOM;
Linting src/wp-admin/js/editor.js ...ERROR
[L33:C17] W117: 'QTags' is not defined.
        QTags.closeAllTags( id );
Linting src/wp-admin/js/editor.js ...ERROR
[L35:C18] W117: 'tinyMCEPreInit' is not defined.
      if ( tinyMCEPreInit.mceInit[ id ] && tinyMCEPreInit.mceInit[ id ].wpautop )
Linting src/wp-admin/js/editor.js ...ERROR
[L35:C50] W117: 'tinyMCEPreInit' is not defined.
      if ( tinyMCEPreInit.mceInit[ id ] && tinyMCEPreInit.mceInit[ id ].wpautop )
Linting src/wp-admin/js/editor.js ...ERROR
[L41:C26] W117: 'tinymce' is not defined.
        ed = new tinymce.Editor( id, tinyMCEPreInit.mceInit[ id ] );
Linting src/wp-admin/js/editor.js ...ERROR
[L41:C46] W117: 'tinyMCEPreInit' is not defined.
        ed = new tinymce.Editor( id, tinyMCEPreInit.mceInit[ id ] );
Linting src/wp-admin/js/editor.js ...ERROR
[L47:C13] W117: 'setUserSetting' is not defined.
      setUserSetting( 'editor', 'tinymce' );
Linting src/wp-admin/js/editor.js ...ERROR
[L58:C22] W117: 'tinyMCEPreInit' is not defined.
        if ( tinyMCEPreInit.mceInit[ id ] && tinyMCEPreInit.mceInit[ id ].wpautop )
Linting src/wp-admin/js/editor.js ...ERROR
[L58:C54] W117: 'tinyMCEPreInit' is not defined.
        if ( tinyMCEPreInit.mceInit[ id ] && tinyMCEPreInit.mceInit[ id ].wpautop )
Linting src/wp-admin/js/editor.js ...ERROR
[L66:C13] W117: 'setUserSetting' is not defined.
      setUserSetting( 'editor', 'html' );
Linting src/wp-admin/js/editor.js ...ERROR
[L1:C18] W098: 'switchEditors' is defined but never used.
var switchEditors = {

"tinyMCE" should be "tinymce", "tinymce" should be declared as a global (as should tinyMCEPreInit, QTags, setUserSetting, and switchEditors). The others (single quotes) should be pretty simple.

#5 @dougwollison
6 years ago

Heh, whoops, thought I reset the repo for that.

As for the jsHint errors, mine wasn't throwing any and I didn't see anything about where the rules file was, took a close look and found it.

New patch coming soon.

#6 follow-up: @dougwollison
6 years ago

Oh, that's why, I'm using the core repo, not the developer repo.

Was that necessary for the inline hook docs? I swear it was suppose to be core.

Last edited 6 years ago by DrewAPicture (previous) (diff)

@dougwollison
6 years ago

Attempt #2; fixed spacing, braces, equals, declared globals (not sure if I used the proper method though).

@dougwollison
6 years ago

Corrected handling of globals. 1 error left; 'switchEditors' is defined but never used

#7 in reply to: ↑ 6 ; follow-up: @DrewAPicture
6 years ago

Replying to dougwollison:

Oh, that's why, I'm using the core repo, not the developer repo.

Was that necessary for the inline hook docs? I swear it was suppose to be core.

There wasn't a problem because core is a packaged version of develop. Going forward if you're generating patches against trunk you should probably use the develop repo.

#8 in reply to: ↑ 7 @dougwollison
6 years ago

Replying to DrewAPicture:

Replying to dougwollison:

Oh, that's why, I'm using the core repo, not the developer repo.

Was that necessary for the inline hook docs? I swear it was suppose to be core.

There wasn't a problem because core is a packaged version of develop. Going forward if you're generating patches against trunk you should probably use the develop repo.

Understood. I'll keep that in mind and replace my local repo.

#9 @azaozz
6 years ago

Some more notes:

  • Don't think defining the globals for JSHint needs the :false part. This is the default.
  • The globals should probably be comma separated on the same line, following the same standard as var.
  • tinymce is a superset of tinyMCE (tinyMCE exists for back-compat), but tinyMCEPreInit is another global that cannot be changed. Please run some tests with the patched file to confirm all is still working.


@dougwollison
6 years ago

Revised declaration of globals, restored case of tinyMCE

#10 follow-up: @dougwollison
6 years ago

I'm a little confused though, do I need to change the case on tinyMCE at all or in only some places?

Wait, I see what you mean; I hand changed it on tinyMCEPreInit.

I'll put up another diff with the tinymce change.

Last edited 6 years ago by dougwollison (previous) (diff)

@dougwollison
6 years ago

Updated to include proper lowercasing of just tinymce

#11 @azaozz
6 years ago

  • Owner set to azaozz
  • Resolution set to fixed
  • Status changed from new to closed

In 26224:

JSHint fixes for editor.js. Props dougwollison, fixes #25947

#12 in reply to: ↑ 10 @azaozz
6 years ago

Replying to dougwollison:

...I hand changed it on tinyMCEPreInit.

Yep, a lot better now :) Thanks.

#13 @dougwollison
6 years ago

Okay, now to just go through my other 10 patches and fix the global declarations on them.

#14 follow-up: @azaozz
6 years ago

No need, whoever commits them can do it.

#15 in reply to: ↑ 14 @dougwollison
6 years ago

Replying to azaozz:

No need, whoever commits them can do it.

Hehe... too late. One or two patches were corrupted anyway.

#16 @SergeyBiryukov
6 years ago

  • Milestone changed from Awaiting Review to 3.8
Note: See TracTickets for help on using tickets.