Bug 332307

Comment 1 Robert Munteanu 2011-05-24 16:58:10 EDT

Any ideas on what might be required to make this work? I am willing to contribute ( I have a basic knowledge of both Eclipse APIs and Javascript ).

By the looks of it, customisation would require injecting some code into base.html, probably by using HtmlComposer.execute(String). I've tried to simulate this, but I can't get access to the CKEditor instance:


bc. composer.executeWithReturn(new GetHtmlCommand()); // ensure initialised
composer.execute("try { alert(CKEDITOR.instances.editor1) } catch (error) { alert(error); }");

I always get ReferenceError: CKEDITOR is not defined .

Comment 2 Tom Seidel 2011-05-24 18:06:35 EDT

Created attachment 196497 [details]
patch

Attached is a first approach how to implement that. To try this out, just create your editor this way:

Configuration config = new Configuration();
config.setEnterMode("BR");
final HtmlComposer composer = new HtmlComposer(comp, SWT.NONE, config);

Comment 3 Tom Seidel 2011-05-24 18:07:28 EDT

Created attachment 196498 [details]
patch2

Please apply both patches

Comment 4 Tom Seidel 2011-05-24 18:10:00 EDT

This is a first idea, which works and has a minimal impact. If you have a better idea, please let me know :)

Comment 5 Robert Munteanu 2011-05-25 02:31:05 EDT

I'll give it a shot, thanks. BTW, there seems to be a typo

bc. 
+	if (enterMode == "P"){
+		config.enterMode = CKEDITOR.ENTER_P;
+	} else if (enterMode = "BR") {
+		config.enterMode = CKEDITOR.ENTER_BR;
+	} else if (enterMode = "BR") {
+		config.enterMode = CKEDITOR.ENTER_DIV;
+	}

I think that:
* the checks should use '=='
* the last check should be against enterMode == 'DIV'

Comment 6 Tom Seidel 2011-05-25 05:36:09 EDT

(In reply to comment #5)
> I'll give it a shot, thanks. BTW, there seems to be a typo
> 
> bc. 
> +    if (enterMode == "P"){
> +        config.enterMode = CKEDITOR.ENTER_P;
> +    } else if (enterMode = "BR") {
> +        config.enterMode = CKEDITOR.ENTER_BR;
> +    } else if (enterMode = "BR") {
> +        config.enterMode = CKEDITOR.ENTER_DIV;
> +    }
> 
> I think that:
> * the checks should use '=='
> * the last check should be against enterMode == 'DIV'

Yes, of course..Was a bit late yesterday ;)

Comment 7 Robert Munteanu 2011-05-26 05:07:27 EDT

I've applied the patch, and the block looks like this now:

bc. 	if (enterMode == "P"){
		config.enterMode = CKEDITOR.ENTER_P;
	} else if (enterMode == "BR") {
		alert("Setting BR mode...");
		config.enterMode == CKEDITOR.ENTER_BR;
	} else if (enterMode == "DIV") {
		config.enterMode = CKEDITOR.ENTER_DIV;
	}	
	alert("From parameter " + enterMode + " set config value to " + config.enterMode + " ( ENTER_BR is " + CKEDITOR.ENTER_BR + " )");
	
The issue is that setting the config.enterMode value does not seem to take effect. The 'Setting BR mode...' alter shows up, but the second alert reads  'From parameter BR set config value to 1 ( ENTER_BR is 2 ) ' . AFAIK ENTER_P has the value 1, so the config.enterMode is not set.

Any idea why?

Comment 8 Robert Munteanu 2011-05-26 05:10:15 EDT

Um ... nevermind . == instead of = .

Comment 9 Tom Seidel 2011-05-26 05:12:23 EDT

change 

config.enterMode == CKEDITOR.ENTER_BR;

to

config.enterMode = CKEDITOR.ENTER_BR;

Comment 10 Robert Munteanu 2011-05-26 05:13:39 EDT

Yup, thanks :-)

So this snippet works just fine.

bc.	if (enterMode == "P"){
		config.enterMode = CKEDITOR.ENTER_P;
	} else if (enterMode == "BR") {
		config.enterMode = CKEDITOR.ENTER_BR;
	} else if (enterMode == "DIV") {
		config.enterMode = CKEDITOR.ENTER_DIV;
	}
	
Do you think that this can enter 0.8 before the according to the project plan ( http://www.eclipse.org/projects/project-plan.php?projectid=mylyn ) - 2011-06-01 Release Candidate (API Freeze) ?

Comment 11 Robert Munteanu 2011-05-26 05:14:16 EDT

s/before the according/before the API freeze according/

Typing is not my strong area today.

Comment 12 Steffen Pingel 2011-05-26 15:13:45 EDT

It's fine to make changes. Note that we'll freeze and branch early next week before RC3.

Comment 13 Tom Seidel 2011-05-27 11:36:57 EDT

(In reply to comment #10)
> 
> Do you think that this can enter 0.8 before the according to the project plan (
> http://www.eclipse.org/projects/project-plan.php?projectid=mylyn ) - 2011-06-01
> Release Candidate (API Freeze) ?

I'll provide another patch tomorrow with the final implementation of setting the BR-Mode and the UseEntitySettings (Implementation-Details will change in comparision to the lateste pateches). Please give me feedback, so that I can push it to the branch before the merging for 0.8 is done. 

I'm not sure if we can implement all the configuration features the underlying ckeditor provides (for a full list see http://docs.cksource.com/ckeditor_api/symbols/CKEDITOR.config.html) but probably you can give me a list of "high-priorized" configuration elements that are important for your usecase.

Comment 14 Robert Munteanu 2011-05-28 03:34:25 EDT

That sounds great. I think that besides these two autoParagraph might be of use.

Comment 15 Tom Seidel 2011-05-28 06:20:24 EDT

Created attachment 196824 [details]
final patch

Third patch. Implemented Entermode, ShiftEnterMode, Entities and Autoparagraph.

Please give me feedback if that patch is suitable for you, so that I can push it.

Here is an example how to use the config-stuff:

Configuration config = new Configuration();
		config.addConfigurationNode(new EnterModeConfiguration(EnterMode.BR));
		config.addConfigurationNode(new ShiftEnterModeConfiguration(EnterMode.P));
		config.addConfigurationNode(new UseEntitiesConfiguration(false));
		final HtmlComposer composer = new HtmlComposer(comp, SWT.NONE, config);

Comment 16 Robert Munteanu 2011-05-28 16:42:21 EDT

Thanks for the updated patch. I am unable to apply the patches in succession - the first two succeed, but the third one fails with:

bc. error: patch failed: org.eclipse.mylyn.htmltext/META-INF/MANIFEST.MF:17
error: org.eclipse.mylyn.htmltext/META-INF/MANIFEST.MF: patch does not apply
Patch failed at 0001 Bug 332307 - Overwriting the default CkEditor configuration

Am am on the 'master' branch and the last commit in it is

bc.. commit 89d44c91d1d2b25b705ff044ca2922dadb344065
Author: Steffen Pingel <steffen.pingel@tasktop.com>
Date:   Wed May 25 15:34:32 2011 +0200

    NEW - bug 345021: update Eclipse SUA for all features
    https://bugs.eclipse.org/bugs/show_bug.cgi?id=345021

Comment 17 Tom Seidel 2011-05-28 18:31:29 EDT

I've pushed the changes now to the repository. Try updating directly from git.

Comment 18 Robert Munteanu 2011-05-29 03:28:42 EDT

Works just fine for me, thanks.

Comment 19 Tom Seidel 2011-05-29 05:46:51 EDT

Ok, marking this as fixed. If other configurations are needed feel free to open a new bug.