Editor CSS problems

[Katie Vance]

​http://download.dojotoolkit.org/release-1.5.0/dojo-release-1.5.0/dijit/themes/themeTester.html?theme=claro&dir=rtl

In RTL, the font drop down has a border next to the arrow where there is no border in LTR. This happens in all themes.

All disabled editors do not show the font drop down arrows in claro, this is not just an RTL problem.

[Katie Vance]

Jared, I attached a patch for this issue.

[bill]

The .dijitEditorDisabled reference is too restricted, since that rule should presumably apply to any disabled ComboBox in a Toolbar, it could be

.claro .dijitToolbar .dijitComboBoxDisabled .dijitArrowButtonInner {
        background-position:0 50%; 
}

[Jared Jurkiewicz]

The problem is a little more complicated than CSS, as it turns out. Though they appear disabled, the dropdowns are not actually disabled. This is because the setDisabledAttr api in editor is firing before all the plugins have loaded, rendering the state 'odd'. IT actually needs to wait until the plugins have all loaded before firing, so it needs to wait on the same deferred as setting the value does, as the editor needs to be basically completely initialized. I'm attaching two patches, one for trunk, one for 1.5.1, that resolves the state load issue. Really need a better disable API for plugins. :)

[Jared Jurkiewicz]

Trunk patch

[Jared Jurkiewicz]

Editor 1.5 patch

[Jared Jurkiewicz]

(In [22884]) Fixing setting of disabled state too early. \!strict refs #11737

[Jared Jurkiewicz]

(In [22885]) Fixing setting of disabled state too early. \!strict fixes #11737

[bill]

screenshot of display problem after [22885]

[liucougar]

no need to check "this.setValueDeferred.fired < 0", if a deferred already fires, addCallback will call the callback immediately

[bill]

Thanks for the work on this. There are some issues though.

1. Like Cougar said:

var disableFunc = ...;
if(this.setValueDeferred && this.setValueDeferred.fired < 0){
	this.setValueDeferred.addCallback(disableFunc);
}else{
	disableFunc();
}

can be simplified to:

this.setValueDeferred.addCallback(...);

(The change to use a Deferred makes sense. I can see in themeTester in 1.5 how the ComboBox is enabled on the (initially) disabled Editor.)

2. the CSS is still incorrect, as I wrote above in comment:3. It needs to apply to all disabled ComboBox's in Toolbars, not just for editor.

3. it looks wrong, there's a white line in the gray background:

4. why does this need to go into the 1.5 branch? Is it a critical regression? It doesn't seem like it.

[Katie Vance]

We not need this fixed in 1.5, it is not critical.

[Jared Jurkiewicz]

It should go into the branch because disabling the editor is effectively broken without the fixes to delay disablement.

Updated css/files in: [22892] and [22891] I could swear I removed that editor css selector several times. Not sure why it showed up yet again (*sigh*)

Reassigning to ksv to continue her work on fixing the white border line.

[Katie Vance]

Attaching css fix for the white line.

[Katie Vance]

screenshot to show onclick event was fired on a disabled combobox

[Katie Vance]

I attached a fix for the extra white line showing up. However we still have an issue with the combo box not getting completely disabled. If you click on the disabled box (not the arrow drop down) it's onclick event is still causing the box to turn white. I'm guessing this should be fixed in the plugin?

This is what it looks like after you click on it: ​http://bugs.dojotoolkit.org/attachment/ticket/11737/afterClick.JPG

[bill]

Katie - presumably after that new patch the changes to dijit_rtl.css in [22884] and [22885] are no longer needed?

Jared - the CSS changes in [22891] and [22892] look incorrect since they are affecting both enabled and disabled ComboBox, aren't they just supposed to be for a disabled ComboBox?

Also, this shouldn't be in 1.5.1. Disabling the editor is working in 1.5.0, even creating an initially disabled editor. (The editor contents are not editable, which is the main point of disabling.) The only issue is that in the latter case (creating an initially disabled editor) the ComboBox is still focusable. That's hardly a critical bug. Is there some other issue?

[bill]

PS: oh maybe the dijit_rtl.css changes are still needed for tundra/soria/nihilo (RTL mode)?

Also, it seems like the !important in that dijit_rtl.css rule will override the border: none in the attachment:eidtor_css_white_line.patch​, are you sure that's working in RTL mode? If the themes are designed to only specify border-color, not border-width, leaving dijit.css and dijit_rtl.css to specify border-width, then probably you don't need !important anywhere.

[Katie Vance]

The patch I sent is not cumulative. All patches should be put together.

Unfortunately we need both border css statements in dijit_rtl.css and Common.css because they are applied to different dom nodes. The Common.css setting applies to the input node because of .dijitArrowButtonInner and the dijit_rtl applies to it's parent node because of .dijitArrowButtonContainer. So the latest patch should be added to the other patches. I've tested the patch in both directions and it is working. The !important is needed to override the other !important setting. The only other option there is to change the border color to be the same as the background (so you can't see it), but that may not be possible without putting a separate rule for every theme.

[Jared Jurkiewicz]

You can click on the dropdown and it does something (Dropdown opens, you can pick a font, etc). It's also likely to invoke 'execCommands' on a disabled editor, which may or may not throw errors (I haven't checked). Having enabled buttons in a disabled editor runs risks of it firing/trying to do something on disabled content.

I feel it's important to fix this in 1.5.1 as well, to avoid possible scenarios as noted above. It isn't just FontChoice? that could be affected, any plugin that isn't properly disabled could trigger bad side-effects.

[Jared Jurkiewicz]

You're right on the other issues (Re, css), I'll look at them when I have time (which I haven't had a lot lately). I'll do what I can.

[Jared Jurkiewicz]

(In [22894]) Applying further css updates. refs #11737

[Jared Jurkiewicz]

(In [22895]) Applying further css updates. refs #11737

[Jared Jurkiewicz]

(In [22896]) Applying further css updates. refs #11737

[Jared Jurkiewicz]

Applied all current patches. I really do fundamentally believe this should be in 1.5.1 as well. The ramifications are more than just FontChoice? (It's just one we see). The failure to properly disable plugin buttons and such in a user-built plugin has the change of causing failures should someone exec one by accident while the editor is in disabled state. The fix for it is minor (using the deferred delay).

[Jared Jurkiewicz]

If after all that I have explained is not enough to convince you, Bill, I'll pull the changes from 1.5.1. I do really believe it's a bigger issue (since it affects more than just combobox), but you are in charge of dijit.

So, if you say pull it, I'll pull it.

[Katie Vance]

Jared, those 3 fixes are good, however, we still have the issue I mentioned above. If you click on the disabled box (not the arrow drop down) it's onclick event is still causing the box to turn white. Should I look to fix this at the plugin level? It happens to all plugins with a drop down box:

[Jared Jurkiewicz]

If you want to, go for it. Also notice that the editor body goes all white too, so I don't think it's strictly the plugin.

• Jared

[Jared Jurkiewicz]

I do have to wonder if removing those 'white' borders is correct. If you look in the themeTester at the form widgets page, the disabled comboboxes have the white outlines.

[Jared Jurkiewicz]

Bill:

Cases that will break without the disable being correct in 1.5:

ViewSource?: If this isn't disabled correctly, toggling it will throw errors/mess up the editor.

FullScreen?: Same as above. If not properly disabled, the code to size out will break.

So any table instantiated with these and initially set disabled will have issues without the fix being in 1.5.

[bill]

Hi Jared - I tried modifying test_ViewSource.html to set the editor initially disabled. After loading the page, enabling the editor (via dijit.byId("editor0").set("disabled", false) and pressing the view source button it seemed to work fine. Can you tell me how to reproduce the problem? (I was trying on FF/mac.)

Katie -OK, I see about those CSS rules being on different nodes, nevermind :-). Thanks for looking at that click problem.

I do have to wonder if removing those 'white' borders is correct. If you look in the themeTester at the form widgets page, the disabled comboboxes have the white outlines.

I see your point, but ComboBox in a toolbar looks quite different from a normal ComboBox (outside of a Toolbar). A vanilla enabled ComboBox has a black separating line, then white padding around the button, then a gray button (with a black arrow), whereas the Editor Toolbar enabled ComboBox has no black line, no white padding, and no grayness for the button. It's all white except for the black arrow. So I figure that the disabled Editor Toolbar ComboBox should also be a single color except for the arrow itself.

Maybe there are some subtleties that I'm not seeing on my monitor.

[Jared Jurkiewicz]

You don't enable it. The WiewSource? will already be enabled because it was not properly disabled. It only looks disabled, but if you hover over it, it shows an active hover box. Furthermore, you can click it.

I tried this, and I could still click on viewSource in the editor and do stuff. In fact, I could chance the editor contents (I had expected errors, but the internals of the editor seem intact enough it doesn't throw). In any event, this shows that editor is not completely disabled.

See attached screenshots.

Without disabled waiting until all plugins have initialized to disable them, you end up with states/cases people can modify/do things they shouldn't be able to. The attached screenshots bear this out.

[Jared Jurkiewicz]

A supposedly disabled editor with active buttons.

[Jared Jurkiewicz]

A disabled editor allowing view source to be activated.

[Jared Jurkiewicz]

A disabled editor, through the use of view source, showing it being altered.

[bill]

Ah I see, OK in that case I'm OK with it.

[Jared Jurkiewicz]

Sorry I wasn't articulating my concerns very well, Bill. After having been in Editor for so long, the issue seemed worse than the defect text stated. Glad I was able to make a clearer case as to why it's pretty bad.

As for the whole click/white thing that ksv is looking at. How does editor mask itself completely grey as it is? It seems to be some sort of focus issue with editor, as clicking on the editor body flips it to whitish too (not just the dropdowns).

[Katie Vance]

It appears that the filteringSelect only turns white in claro. The editor turning white happens in all themes.

[Jared Jurkiewicz]

Reassigning back to ksv since she is working on the 'white when clicked' issue.

[Katie Vance]

Attached patch for the problems with the input box and the editor turning white when a user clicks on the disabled drop down plugin.

[Katie Vance]

previous patch was corrupted, try this one

[Jared Jurkiewicz]

(In [22933]) Fix for the editor going white in claro. refs #11737

[Jared Jurkiewicz]

(In [22934]) Final bit of the fix for disabling the editor. Claro issue resolved. fixes #11737