101800 - XUL <label value="something"> does not support text-transform: uppercase

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[Mike Potter]

XUL should support CSS "text-transform: uppercase" so the OEone design team can
get off my back for not having the month names all caps in our calendar.  This
function is supported for HTML.  I have a test case which I will attach.

Comment 1

[Mike Potter]

Attachment: XUL Testcase for text-transform: uppercase

Comment 2

[Mike Potter]

From an eamil I received from bz: 
<label style="text-transform:uppercase">text</label> works by the way.  So it's
just the replaced-element version that uses value="stuff" that fails...

Comment 3

[Peter Bojanic]

Nominating for mozilla1.0

Comment 4

[David Hyatt]

--> default owner

Comment 5

[Daniel Kirsch]

Any progress on this? Should be not to hard to implement. Workarounds?

Comment 6

[Daniel Kirsch]

My current workaround in a global css file:

textbox input {
  text-transform:inherit;
}

Looks like text-transform is predefined somewhere.

Comment 7

[Daniel Kirsch]

Looking into the "toolkit" and "classic skin" sources, I cannot find a text-transform definition.

Probably the most easiest fix would be to add the workaround from comment #6 to the xul.css document in "/toolkit/content/global/xul.css"

Comment 8

[Dão Gottwald [:dao]]

I don't see what your workaround has to do with this bug.

Comment 9

[Daniel Kirsch]

(In reply to comment #8)
> I don't see what your workaround has to do with this bug.

It fixes it. At least for xul:textbox elements.
With that global definition, you may now use

textbox {
  text-transform:uppercase;
}

and it should work as expected.

Daniel

Comment 10

[Daniel Kirsch]

The reason is, that a xul:textbox element is just a xbl widget based on a html:input element.

Well, it's also based on html:textarea when setting the multiline attribute to "true" so my workaround might be extended to handle that as well.

For a reason I still didn't found, that html element doesn't inherit the defined style from it's parent which actually is the textbox element itself. (According to the CSS spec, text-transform should initially be inherited. => http://www.w3.org/TR/CSS21/text.html#propdef-text-transform )

Comment 11

[Dão Gottwald [:dao]]

(In reply to comment #9)
> (In reply to comment #8)
> > I don't see what your workaround has to do with this bug.
> 
> It fixes it.

It doesn't. Please take a look at the testcase.

(In reply to comment #10)
> The reason is, that a xul:textbox element is just a xbl widget based on a
> html:input element.

Exactly, but that's not what this bug is about.

Comment 12

[Daniel Kirsch]

Yes, you are right. It doesn't fix the general problem.
The workaround is for textboxes only but that's probably a very typicall situation as for a label, I can controll the content easily, while for an user input I cannot.

As a label (like the testcase uses it) is not based on a HTML element you may create your own binding as a workaround inheriting from xul:label and adding a html element inside your xbl:content element.

Comment 13

[Dão Gottwald [:dao]]

(In reply to comment #12)
> As a label (like the testcase uses it) is not based on a HTML element you may
> create your own binding as a workaround inheriting from xul:label and adding a
> html element inside your xbl:content element.

The preferable workaround is to use real text content; see comment 2.

Comment 14

[Daniel Kirsch]

(In reply to comment #13)
> The preferable workaround is to use real text content

Of course. But for larger projects this means to update each relevant label element and to change the way you set a label dynamically.
Usually you just use:

labelElm.value = 'foo';

When using a textNode you need to do something like:

var labelElm=document.getElementById('text');
while(labelElm.firstChild)
  labelElm.removeChild(labelElm.firstChild);
labelElm.appendChild(document.createTextNode('foo'));

So it depends on your usage and what's easier.

Comment 15

[Dão Gottwald [:dao]]

(In reply to comment #14)
> When using a textNode you need to do something like:
> 
> var labelElm=document.getElementById('text');
> while(labelElm.firstChild)
>   labelElm.removeChild(labelElm.firstChild);
> labelElm.appendChild(document.createTextNode('foo'));

labelElm.textContent = "foo";

Comment 16

[Daniel Kirsch]

(In reply to comment #15)
> labelElm.textContent = "foo";

...which is much better. Didn't knew it's usable for XUL too.

Comment 17

[Pardal Freudenthal (:ShareBird)]

Any news on this? Also the CSS properties: word-spacing and letter-spacing don't work on label elements.

Comment 18

[D7460N]

Please advise as to the status of XUL label elements supporting CSS properties text-transform, letter-spacing, and word-spacing. Thank you.

Comment 19

[Tim Nguyen :ntim]

Is there any possible CSS workaround we can use for the time being ? We need this for bug 1011014 . I tried to look for some kind of anonymous element, but I couldn't find the relevant one. I did find a span anonymous element, but it's for the underlined accesskey.

Comment 21

[Boris Zbarsky [:bzbarsky]]

This has nothing to do with CSS.

The basic issue is that <label value="something"> doesn't use the normal text rendering codepath.  So anything implemented in textframe code won't work there: text-transform, selection drawing, letter/word spacing, hyphenation, line-breaking, etc.

You can use <label>something</label>, per comment 2, to get normal text rendering...

Comment 22

[Boris Zbarsky [:bzbarsky]]

Note that I suspect doing uppercase/lowercase transforms here would not be too bad.  Supporting "capitalize" would take more work.

Comment 23

[Tim Nguyen :ntim]

(In reply to Boris Zbarsky [:bz] from comment #21)
> This has nothing to do with CSS.
> 
> The basic issue is that <label value="something"> doesn't use the normal
> text rendering codepath.  So anything implemented in textframe code won't
> work there: text-transform, selection drawing, letter/word spacing,
> hyphenation, line-breaking, etc.
> 
> You can use <label>something</label>, per comment 2, to get normal text
> rendering...

Do you think you can mentor me through this bug ? Note that I've never touched XUL/HTML/JS/CSS rendering stuff, and I don't know C++ at all. Is my JS and CSS knowledge enough for this bug ?

Comment 24

[Boris Zbarsky [:bzbarsky]]

Attachment: Add support for text-transform:uppercase/lowercase (but not other values) on <xul:label value="whatever">

As long as people are OK with just supporting uppercase/lowercase, this should do it...

Comment 25

[Boris Zbarsky [:bzbarsky]]

Sorry, I didn't see comment 23 until after I wrote the patch.  :(

Is what it does what you wanted to be mentored through?  If not, what changes did you have in mind?  Making this code use the normal text rendering code is probably doable, but Robert O'Callahan may be a better mentor than me for that.

Comment 26

[Daniel Holbert [:dholbert]]

Comment on attachment 8426591 [details] [diff] [review]
Add support for text-transform:uppercase/lowercase (but not other values) on <xul:label value="whatever">

This probably needs a reftest, I imagine?
>+void
>+nsTextBoxFrame::RecomputeTitle()
>+{
>+  mContent->GetAttr(kNameSpaceID_None, nsGkAtoms::value, mTitle);
>+
>+  uint8_t textTransform = StyleText()->mTextTransform;
>+  if (textTransform == NS_STYLE_TEXT_TRANSFORM_UPPERCASE) {
>+    ToUpperCase(mTitle);
>+  } else if (textTransform == NS_STYLE_TEXT_TRANSFORM_LOWERCASE) {
>+    ToLowerCase(mTitle);
>+  }

So, I looked at other places where we react to these values. One such place is nsCaseTransformTextRunFactory::RebuildTextRun(), and it does different things depending on StyleFont()->mLanguage (via a local var "languageSpecificCasing").
http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsTextRunTransformations.cpp?rev=5a9badd6db00#788

Do we need something like that here? (Maybe that's not necessary, since this is already intentionally-incomplete. Still, might be worth at least adding a comment noting that this doesn't handle language-specific capitalization rules. Up to you.)

>+  // We can't handle NS_STYLE_TEXT_TRANSFORM_CAPITALZIE because we
>+  // have no clue about word boundaries here.

s/CAPITALZIE/CAPITALIZE/

>+void
>+nsTextBoxFrame::DidSetStyleContext(nsStyleContext* aOldStyleContext)
>+{
[...]
>+  const nsStyleText* oldTextStyle = aOldStyleContext->PeekStyleText();
>+  if (oldTextStyle &&
>+      oldTextStyle->mTextTransform != StyleText()->mTextTransform) {
>+    RecomputeTitle();
>+    UpdateAccessTitle();
>+  }

So if PeekStyleText() returns null here, we'll skip the update.  But shouldn't we really be assuming the worst (that text-transform has changed) and proceeding with the update, for correctness?

Comment 27

[Daniel Holbert [:dholbert]]

(Note that there's also one more value for text-transform -- "full-width"[1] -- which we support[2] in general, and presumably could support here, though I'm not sure whether it's worth it. Probably worth at least mentioning it in a comment, for completeness, since otherwise we're handling or explaining away all of the values except for that one.)

[1] http://dev.w3.org/csswg/css-text/#text-transform-property
[2] bug 774560

Comment 28

[Boris Zbarsky [:bzbarsky]]

> This probably needs a reftest, I imagine?

Yeah, I should add one.

> Do we need something like that here?

I don't know.  I modeled this code after nsTextFrame::GetRenderedText, which doesn't do anything with the language.  But you're right that the normal textrun handling seems to do something fancier.  I'll add a comment.

> But shouldn't we really be assuming the worst

If PeekStyleText() returns null that means no one ever called StyleText() on aOldStyleContext, which means we don't care about any changes to anything in that struct.

I could take that null-check out, I guess, since nsTextBoxFrame::Init will end up calling RecomputeTitle via the UpdateAttributes() call it makes, so in practice we will never get a null PeekStyleText()...

> and presumably could support here

Ah, I guess we could.  I'll document that we don't.  Neither does nsTextFrame::GetRenderedText.

Comment 29

[Boris Zbarsky [:bzbarsky]]

Attachment: Add support for text-transform:uppercase/lowercase (but not other values) on <xul:label value="whatever">

Comment 30

[Tim Nguyen :ntim]

(In reply to Boris Zbarsky [:bz] from comment #25)
> Sorry, I didn't see comment 23 until after I wrote the patch.  :(
> 
> Is what it does what you wanted to be mentored through?  If not, what
> changes did you have in mind?  Making this code use the normal text
> rendering code is probably doable, but Robert O'Callahan may be a better
> mentor than me for that.

It's OK, you can take this bug, you seem much better for this job than I am.

Comment 31

[Boris Zbarsky [:bzbarsky]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/67fe1ff27cad

Comment 32

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/67fe1ff27cad