Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#11528 closed defect (fixed)

[regression] TextBox: font-family becomes difficult to override in IE

Reported by: Kenneth G. Franqueiro Owned by: Douglas Hays
Priority: high Milestone: 1.5.1
Component: Dijit - Form Version: 1.5
Keywords: TextBox Cc:
Blocked By: Blocking:

Description (last modified by Douglas Hays)

In Dojo 1.5 (as of revision 21560), TextBox has its font-family style set inline on IE. This causes enormous potential for custom-CSS-clobbering regressions, especially in the case of TextBoxes constructed without srcNodeRef.

To summarize:

In cases where srcNodeRef is not specified, IE's default style will end up inlined, only overrideable by an !important rule.

In cases where srcNodeRef is specified, if the source node matches a selector having a font-family rule, it will be inlined. However, if the font-family is supplied on .dijitTextBox, it will be clobbered, since this class is added during _Widget's postCreate which is not called until AFTER the inlining is done by TextBox.

This is probably much more efficiently illustrated with an example, which I have uploaded. In this test page, any textbox whose font-family is not effectively monospace has failed the test. Please see the example's source for more details.

Attachments (2)

test_TextBoxRegression_fontFamily.html (3.3 KB) - added by Kenneth G. Franqueiro 9 years ago.
Example of bug with test cases
11528.patch (1.3 KB) - added by Douglas Hays 9 years ago.
possible fix that adds setTimeout to fontFamily munging

Download all attachments as: .zip

Change History (12)

Changed 9 years ago by Kenneth G. Franqueiro

Example of bug with test cases

comment:1 Changed 9 years ago by bill

Cc: Douglas Hays removed
Owner: set to Douglas Hays
Summary: [regression] font-family of TextBox becomes difficult to override in IE[regression] TextBox: font-family becomes difficult to override in IE

That's strange, postCreate() has IE specific code to copy the font, to make it work:

if(dojo.isIE){ // IE INPUT tag fontFamily has to be set directly using STYLE
	var s = dojo.getComputedStyle(this.domNode);
	if(s){
		var ff = s.fontFamily;
		if(ff){
			var inputs = this.domNode.getElementsByTagName("INPUT");
			if(inputs){
				for(var i=0; i < inputs.length; i++){
					inputs[i].style.fontFamily = ff;
				}
			}
		}
	}
}

You need to set font on the DOMNode, but that appears to be what the test case is doing. The only thing that changed from 1.4 to 1.5 was to make the TextBox template similar to the ValidationTextBox template, having a <div> surrounding the <input>, so I think whatever problem you are seeing now with TextBox must have been there previously with ValidationTextBox (but tell me if I'm wrong).

Doug - BTW that special-case code can be reduced to if(dojo.isIE < 8 || (dojo.isIE && dojo.isQuirks)). I guess eventually we can remove it altogether, when we decide that IE6/7 aren't important enough to worry about the font-family for inputs.

comment:2 Changed 9 years ago by bill

Oh I didn't read kgf's description carefully enough, it fails because getComputedStyle() is called before the CSS rule is effective, either because the TextBox is created without srcNodeRef specified, or since the dijitTextBox class is applied after the getComputedStyle() call.

Perhaps that IE workaround code is doing more harm than good, and should be removed. We could tell users to make CSS rules like this, that will work on all browsers:

.dijitTextBox INPUT { ... }

comment:3 Changed 9 years ago by Douglas Hays

Is this really a regression? I ran the above test using 1.4 and none the fontFamily's were set to monospace, ignoring the user-styling altogether. So it fails differently in 1.5. Apparently they all received sans-serif from the tundra/form/Common.css rule. So I'm assuming that monospace should be the correct fontFamily in each of the test's 4 TextBox? widgets. Adding bill's IE8/strict suggestion is bad since that makes IE8 report cursive as the fontFamily. IE8 still has issues with font:inhreit not working. During postCreate, IE is apparently still fixing up the fontFamily. Doing the munging in a setTimeout/0 sets monospace in all browsers (strict and quirks), which I think is the best solution for the end-user. kgf, please review the attached patch.

Changed 9 years ago by Douglas Hays

Attachment: 11528.patch added

possible fix that adds setTimeout to fontFamily munging

comment:4 Changed 9 years ago by bill

Description: modified (diff)

Perhaps the "regression" is merely that TextBox widgets now share the same problems as ValidationTextBox widgets regarding setting the font on IE. That's just the price we paid for changing the TextBox template to place nice with the other form widgets.

In 2.0 we'll keep things simple, desupporting using style="..." as a way to set the font of a *TextBox, and instead tell developers to either set it through a CSS rule, or perhaps have a custom parameter like inputStyle="font: ...".

However, for 1.x maybe the setTimeout() is a good compromise, so that style="..." keeps working in general. Note that the setTimeout() fix won't work if an app does a

var x = new dijit.form.TextBox();

and then sometime later, after the setTimeout() triggers, attaches that TextBox to the DOM. Luckily that's won't occur often.

comment:5 Changed 9 years ago by Douglas Hays

Description: modified (diff)
Milestone: tbd1.5.1
Status: newassigned

comment:6 Changed 9 years ago by Douglas Hays

Resolution: fixed
Status: assignedclosed

(In [22742]) Fixes #11528. Add setTimeout to the IE-fontFamily-munging to give user class styling time to be applied.

comment:7 Changed 9 years ago by Douglas Hays

(In [22743]) Refs #11528. Backport [22742] to 1.5.1

comment:8 Changed 9 years ago by Kenneth G. Franqueiro

Seems like the setTimeout workaround fixes it in the tests I supplied, tried in IE8 and IE6. I can't say I'm a big fan of such a workaround, and as Bill said it doesn't cover 100% of scenarios, but I guess it's the simplest way to achieve close to the previous functionality.

Meanwhile, is there a reason you added font-family: Courier to BODY DIV in that test file? Kinda makes the whole thing look a bit crazy.

comment:9 Changed 9 years ago by Kenneth G. Franqueiro

(In [23808]) Removing Courier font-family set on body in test_validate to preserve look and feel from theme; added two more tests to test font-family inheritance on textboxes in a single div instead. Refs #11528

comment:10 Changed 9 years ago by bill

Component: DijitDijit - Form
Note: See TracTickets for help on using tickets.