Opened 8 years ago

Closed 8 years ago

Last modified 6 years ago

#2685 closed New Feature (fixed)

Integrate the SpellChecker.net "Web Spell Checker"

Reported by: fredck Owned by: fredck
Priority: Normal Milestone: FCKeditor 2.6.4
Component: UI : Spell Checker Version:
Keywords: Confirmed Review+ Cc: SpellChecker.net

Description

We are partnering with SpellChecker.net to bring a nice new feature to the editor. It consists on a powerful spell checker (and not only) that requires no installation neither in the browser nor in the server.

This spell checker is called "Web Spell Checker". It's an online service provided by SpellChecker.net, which is closely integrated with the editor interface. It is free, but the free version displays a banner space while the spell check dialog is open.

Because of the "zero installation" nature of this solution, it is supposed to become the default spell checker in the editor. We'll still support SpellerPages and ieSpell just like before.

This ticket should handle not only the code necessary to introduce this feature in the editor, but also all documentation changes and additions to properly inform our users about the possible settings, and an explanation for the banner thing.

Attachments (4)

2685.patch (12.4 KB) - added by fredck 8 years ago.
icon.gif (937 bytes) - added by fredck 8 years ago.
Proposed icon change.
2685_2.patch (13.9 KB) - added by fredck 8 years ago.
2685_3.patch (13.9 KB) - added by fredck 8 years ago.

Download all attachments as: .zip

Change History (15)

Changed 8 years ago by fredck

Changed 8 years ago by fredck

Proposed icon change.

comment:1 Changed 8 years ago by fredck

  • Status changed from new to assigned

A first patch has been attached. It should be already work, using the SpellChecker.net web services.

SpellChecker.net is also proposing a new Spell Check icon, to replace the current one. It's also attached.

I'm not asking for review yet, as we are still controlling the code with SpellChecker.net.

comment:2 Changed 8 years ago by fredck

  • Keywords Review? added

comment:3 follow-up: Changed 8 years ago by alfonsoml

  • Keywords Review- added; Review? removed

The dialog is reading the data with GetData() but then it sets it back with innerHTML, so any code protection won't be executed.

The coding style doesn't follow the guidelines, and there are several spelling mistakes in the comments :-)
There are several debugging statements commented out, I don't see any reason to keep them in the production code.

I'm not sure if the code should use the "default" case in FCKSpellCheckCommand.prototype.Execute or if it should work only when the value is "WSC" (and also the IsEnabled property)

The code uses frames with src="", doesn't that generate problems when using https? (I'm not sure). Anyway, the user will get a warning due to the load of http://loader.spellchecker.net from a secure page.

Changed 8 years ago by fredck

comment:4 in reply to: ↑ 3 Changed 8 years ago by fredck

  • Keywords Review? added; Review- removed

Ok... a new patch has been added with several modifications.

Replying to alfonsoml:

The dialog is reading the data with GetData() but then it sets it back with innerHTML, so any code protection won't be executed.

I've changed it to use SetData instead.

The coding style doesn't follow the guidelines, and there are several spelling mistakes in the comments :-)

I've fixed most of the code style. (this code has been provided by SpellChecker.net)

There are several debugging statements commented out, I don't see any reason to keep them in the production code.

Cleaned up.

I'm not sure if the code should use the "default" case in FCKSpellCheckCommand.prototype.Execute or if it should work only when the value is "WSC" (and also the IsEnabled property)

True... it's not anymore a default case. No changes for IsEnabled instead, as our currently implementation means that FF and similars are not compatible with ieSpell, not that they are compatible with a set of option.

The code uses frames with src="", doesn't that generate problems when using https? (I'm not sure). Anyway, the user will get a warning due to the load of http://loader.spellchecker.net from a secure page.

I've tested it over https. It works well, but it gives the "mixed content" warning. Not a big issue for the beta, but we'll try to fix it for the stable. I'll file a ticket for it, as soon as this ticket get closed.

comment:5 Changed 8 years ago by martinkou

  • Keywords Review- added; Review? removed

w.html, line 146: If initAndSpell() has been called, then this check shouldn't be done at all. w.html, line 109, 119, 121, 171, 199, 216: Still need to correct bracket styles. tmpFrameset.html, line 33, 36, 43, 50, 51: Bracket styles. ciframe.html, line 48, 55: Bracket styles. fckspellcheckcommand_gecko.js, line 28: Recommend it to be changed to

this.IsEnabled = FCKConfig.SpellChecker.Equals( [ 'WSC', 'SpellerPages' ] );

fckspellcheckcommand_ie.js, line 28: Recommend it to be changed to

this.IsEnabled = FCKConfig.SpellChecker.Equals( [ 'WSC', 'SpellerPages', 'ieSpell' ] );

Changed 8 years ago by fredck

comment:6 Changed 8 years ago by fredck

Attached a new patch with the code style changes.

As said in my previous comments, the patch implements isEnable in a way that it specifies that ieSpell is not compatible with Firefox. Any other value is accepted as compatible with all browsers. This is intentional. The code is shorter... and we can't apply error check around all the code in JavaScript, unfortunately.

comment:7 Changed 8 years ago by martinkou

  • Keywords Review+ added; Review- removed

comment:8 Changed 8 years ago by fredck

Ok... awaiting for the SpellChecker.net review to commit.

comment:9 Changed 8 years ago by SpellChecker.net

Review+ by SpellChecker.net

We have reviewed the patch 2685_3.patch and did not found any problem.

comment:10 Changed 8 years ago by fredck

  • Cc SpellChecker.net added
  • Resolution set to fixed
  • Status changed from assigned to closed

Fixed with [2907].

comment:11 Changed 8 years ago by fredck

The new icon has been introduced with [2909].

Note: See TracTickets for help on using tickets.
© 2003 – 2016 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy