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

RESOLVED FIXED in mozilla32

Status

()

RESOLVED FIXED
17 years ago
5 years ago

People

(Reporter: mikeypotter, Assigned: bzbarsky)

Tracking

Trunk
mozilla32
x86
Linux
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

17 years ago
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.
(Reporter)

Comment 1

17 years ago
Created attachment 50947 [details]
XUL Testcase for text-transform: uppercase
(Reporter)

Updated

17 years ago
Keywords: oeone

Updated

17 years ago
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.0

Updated

17 years ago
Target Milestone: mozilla1.0 → mozilla1.0.1
(Reporter)

Comment 2

17 years ago
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

17 years ago
Nominating for mozilla1.0
Keywords: mozilla1.0

Comment 4

16 years ago
--> default owner
Assignee: hyatt → jaggernaut
Status: ASSIGNED → NEW
Target Milestone: mozilla1.0.1 → ---

Comment 5

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

Comment 6

12 years ago
My current workaround in a global css file:

textbox input {
  text-transform:inherit;
}

Looks like text-transform is predefined somewhere.

Comment 7

12 years ago
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

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

Updated

11 years ago
Attachment #50947 - Attachment mime type: text/plain → application/vnd.mozilla.xul+xml

Updated

11 years ago
QA Contact: jrgmorrison → xptoolkit.widgets

Comment 9

11 years ago
(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

11 years ago
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 )
(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

11 years ago
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.
(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

11 years ago
(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.
(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

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

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

Updated

10 years ago
Assignee: jag → nobody
Any news on this? Also the CSS properties: word-spacing and letter-spacing don't work on label elements.

Comment 18

6 years ago
Please advise as to the status of XUL label elements supporting CSS properties text-transform, letter-spacing, and word-spacing. Thank you.
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.
Flags: needinfo?(bzbarsky)
Duplicate of this bug: 1011870
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...
Flags: needinfo?(bzbarsky)
Summary: XUL does not support text-transform: uppercase → XUL <label value="something"> does not support text-transform: uppercase
Note that I suspect doing uppercase/lowercase transforms here would not be too bad.  Supporting "capitalize" would take more work.
(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 ?
Flags: needinfo?(bzbarsky)
Created attachment 8426591 [details] [diff] [review]
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...
Attachment #8426591 - Flags: review?(dholbert)
(Assignee)

Updated

5 years ago
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
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.
Flags: needinfo?(bzbarsky)
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?
(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
> 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.
Created attachment 8426792 [details] [diff] [review]
Add support for text-transform:uppercase/lowercase (but not other values) on <xul:label value="whatever">
Attachment #8426792 - Flags: review?(dholbert)
(Assignee)

Updated

5 years ago
Attachment #8426591 - Attachment is obsolete: true
Attachment #8426591 - Flags: review?(dholbert)
(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.
Attachment #8426792 - Flags: review?(dholbert) → review+
https://hg.mozilla.org/mozilla-central/rev/67fe1ff27cad
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
You need to log in before you can comment on or make changes to this bug.