Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#10096 closed defect (fixed)

queryCommandValue + hiliteColor

Reported by: emorvill Owned by: Jared Jurkiewicz
Priority: high Milestone: 1.4
Component: Editor Version: 1.3.2
Keywords: hilitecolor Cc: bill, liucougar
Blocked By: Blocking:

Description

A queryCommandValue with a hiliteColor doesn't work.

In dijit._editor.plugins.TextColor?, if you overwrite the method updateState like this :

updateState: function(){
	var _e = this.editor;
	var _c = this.command;
	if(!_e || !_e.isLoaded || !_c.length){ return; }
	if(this.button){
		var value;
		try{
			value = _e.queryCommandValue(_c);
		}catch(e){
			console.error(e);
		}
		console.warn("Update State ::: ", _c, " : ", value);
	}
}

value is always equals to "transparent".

Attachments (4)

test_Editor.html (2.1 KB) - added by emorvill 10 years ago.
Test for hilite color
patch_hiliteColor.txt (661 bytes) - added by emorvill 10 years ago.
Patch hiliteColor
test_Editor.2.html (2.2 KB) - added by Jared Jurkiewicz 10 years ago.
Updated over-ride that works with current editor.
possible.patch (782 bytes) - added by Jared Jurkiewicz 10 years ago.
Another possible patch that solves it without altering the default value the editor is using for styleWithCSS

Download all attachments as: .zip

Change History (17)

comment:1 Changed 10 years ago by Adam Peller

Component: DijitEditor
Milestone: 1.3.3tbd
Owner: set to Jared Jurkiewicz

comment:2 Changed 10 years ago by Adam Peller

does this happen for all browsers and all test cases?

comment:3 Changed 10 years ago by bill

I'm unclear what the actual bug (or ramifications of the bug) are. That the hilite-color and background-color buttons don't reflect the current state of the text where the caret is?

See also #5829.

comment:4 Changed 10 years ago by emorvill

Hi,

I made a test : http://volantis.ccett.fr/test_hilitecolor/dijit/tests/test_Editor.html

Firefox 3.x :

  • The hiliteColor always returns the value transparent
  • The foreColor works and returns an hexadecimal value.

IE 6/7/8 :

  • The hiliteColor works and returns an Integer value.
  • The foreColor works and returns an Integer value.

Safari 4 /Chrome :

  • The hiliteColor works and returns a rgb/rgba value (rgb(135,41,41) for example)
  • The foreColor works and returns a rgb/rgba value (rgb(135,41,41) for example)

I think that the bug is on Firefox for hiliteColor command. (It seems impossible to get the right value). May be it will be easier to always returns an hexadecimal value in all browsers ?

Changed 10 years ago by emorvill

Attachment: test_Editor.html added

Test for hilite color

Changed 10 years ago by emorvill

Attachment: patch_hiliteColor.txt added

Patch hiliteColor

comment:6 Changed 10 years ago by emorvill

The problem is when useCSS is set to false after the execCommand. With the patch, I get the right value on Firefox.

comment:7 Changed 10 years ago by Jared Jurkiewicz

I'm not sure that patch is appropriate, as it is going to change a lot of default behavior. In other words, the styling of all elements is going to change. That may have far-reaching effects to other actions.

comment:8 Changed 10 years ago by Jared Jurkiewicz

I'm assuming that updateState method is yours, if so, then why not just implement it like:

updateState: function(){
	var _e = this.editor;
	var _c = this.command;
	if(!_e || !_e.isLoaded || !_c.length){ return; }
	if(this.button){
		var value;
		try{
                        if(dojo.isMoz){
                           e.execCommand("styleWithCSS", false, true);
      			   value = _e.queryCommandValue(_c);
                           e.execCommand("styleWithCSS", false, false);
                        }else{
                           value = _e.queryCommandValue(_c);
                        }
		}catch(e){
			console.error(e);
		}
		console.warn("Update State ::: ", _c, " : ", value);
	}
}

That avoids having to change the default editor behavior (been around for years), for something you desire to over-ride.

Changed 10 years ago by Jared Jurkiewicz

Attachment: test_Editor.2.html added

Updated over-ride that works with current editor.

comment:9 Changed 10 years ago by emorvill

Hi,

It works with this code :

updateState: function(){
	var _e = this.editor;
	var _c = this.command;
	if(!_e || !_e.isLoaded || !_c.length){ return; }
	if(this.button){
		var value;
		try{
                        if(dojo.isMoz){
                          _e.document.execCommand("styleWithCSS", false, true);
      			   value = _e.queryCommandValue(_c);
                          _e.document.execCommand("styleWithCSS", false, false);
                        }else{
                           value = _e.queryCommandValue(_c);
                        }
		}catch(e){
			console.error(e);
		}
		console.warn("Update State ::: ", _c, " : ", value);
	}
}

I think that the queryCommandValue has to work fine without specific code. Maybe, the problem could be resolved in a future version ? I let you decide ;) Thanks for your support

comment:10 Changed 10 years ago by Jared Jurkiewicz

Cc: bill wildbill liucougar added

I added a testcase that alters your over-ride and gets it to work without making any changes to RichText?.js. I'm loathe to change the editor from named tags to CSS styling in 1.X, as it has been the normal editor behavior for years.

Bill, Liu,

Any opinions here? I can change editor over to do styleWithCSS true, but it alters a long-standing behavior. I'm worried what impact such a change might have.

Changed 10 years ago by Jared Jurkiewicz

Attachment: possible.patch added

Another possible patch that solves it without altering the default value the editor is using for styleWithCSS

comment:11 Changed 10 years ago by Jared Jurkiewicz

Resolution: fixed
Status: newclosed

(In [20573]) Minor tweak to get this to function on Moz based browers. \!strict fixes #10096

comment:12 Changed 10 years ago by bill

Cc: wildbill removed
Milestone: tbd1.4

Yah, I don't see any reason to change the editor's behavior in general. I'm sure it's a religious issue that some people feel <b> and <strong> are evil but I don't really care.

I would like to have editor output be consistent across browsers but we can do that with pre/post filters (both DOM based and regex based) so I don't see a pressing need to alter the representation of styles while the document is being edited.

comment:13 Changed 10 years ago by Jared Jurkiewicz

(In [20612]) Used wrong varname, fixed some tabs/spaces mixing. \!strict refs #10096

Note: See TracTickets for help on using tickets.