A number of useful features for Mozilla would be best implemented using underlining effects which are more varied than CSS's awkward `text-decoration' property provides for. These features include: * a different underline style for links which will open in a new window <bug 14027> * a different underline style for links which are to internal anchors in the same document <bug 58135> * a dotted underline for elements with TITLE attributes <bug 56702> * a spell-as-you go feature in Composer which gives misspelled words a red zigzag underline as the user types them. So, I suggest a `-moz-text-underline' property and its children be introduced. Straw man spec as follows. `-moz-text-underline' Value: none | [ <`-moz-text-underline-width'> || <`-moz-text-underline-style'> || <`-moz-text-underline-color'> ] | inherit Initial: none Applies to: all elements Inherited: no (see prose) Percentages: N/A Media: visual This property describes underlining that is added to the text of an element. Unlike the `text-decoration' property, it allows for underlining which is a different color from the text of an element, it allows for a number of different underlining styles, and it allows for underlining which is not mutually exclusive with overlining or other text decorations. If the property is specified for a block-level element, it affects all inline-level descendants of the element. If it is specified for (or affects) an inline-level element, it affects all boxes generated by the element. If the element has no content or no text content (e.g., the IMG element in HTML), user agents must ignore this property. This property is not inherited, but descendant boxes of a block box should be formatted with the same underlining. The color of underlining should remain the same even if descendant elements have different 'color' values. The height of an element should be increased, where necessary, to make room for underlining added to text in that element. `-moz-text-underline-width' [as for `border-top-width'] `-moz-text-underline-style' Value: none | solid | double | dotted | dashed | wavy | inherit Initial: none Applies to: all elements Inherited: no (see note for `-moz-text-underline') Percentages: N/A Media: visual The 'border-style' property sets the style of the underlining for the element. The values have the following effects: none No underlining. solid The underline is a single line segment. double The underlining consists of two solid lines, each with a width equal to the value of `-moz-text-underline-width', with a space between them which also has a width equal to the value of `-moz-text-underline-width'. (Note that this interaction between the `-moz-text-underline-width' property and the `double' value for the `-moz-text-underline-style' property, is different from the interaction between the `border-width' property and the `double' value for the `border-style' property.) dotted The underline is a series of dots. dashed The underline is a series of short line segments. wavy The underline is a curved or zig-zagged line. Conforming lizards may interpret any or all of `dotted', `dashed', `double', and `wavy' as `solid', but it would be nice if they didn't.

Errr ... I omitted out the spec for `-moz-text-underline-color', but it should be pretty obvious.

This is very close to the CSS3 proposal for 'text-underline-style', 'text-overline-style' and the related properties thereon (mode,position,color). The CSS3 proposal includes a few more styles (thick, single-accounting, double-accounting, dot-dash, dot-dot-dash) but does not suggest width as a separate property (I think it probably should). If we decide to implement this as -moz-properties before the spec is public, then we should probably try to stick with the details of the CSS3 proposal.

Netscape's standard compliance QA team reorganised itself once again, so taking remaining non-tables style bugs. Sorry about the spam. I tried to get this done directly at the database level, but apparently that is "not easy because of the shadow db", "plus it screws up the audit trail", so no can do...

A working draft of the CSS3 text module was recently made public: http://www.w3.org/TR/css3-text/#text-underline-props

-width still doesn't exist at W3C, but they have now: text-underline-color (the color) text-underline-style (values: none | solid | double | dotted | thick | dashed | dot-dash | dot-dot-dash | wave) text-underline-position (this is btw. implemented in MSIE but totally broken) text-underline-mode (should spaces be underlines) and a shorthand text-underline for all this. same exists for text-overline and text-line-through

See also bug 127962, which might add -moz-dashed-underline for accessibility until this CSS3 draft is stable enough to implement.

Assigning pierre's remaining Style System-related bugs to myself.

Thai example & Hindi example also other RFEs related to "text-decoration-mode" (like "skip cell") bug 156881

The css3 text module is a candidate recommendation since May 14 -> tweaking summary. http://www.w3.org/TR/css3-text/#text-decoration-overview

I would love to see the double underline support using : text-underline-style: double; This would be useful in transcribing the Chinese Bible, where place names are represented with the double underline, whereas person names are represented by single underline. See http://www.w3.org/TR/css3-text/#text-decoration-style 9.2. Text decoration style: the 'text-underline-style', 'text-line-through-style' and 'text-overline-style' properties Names: text-underline-style, text-line-through-style, text-overline-style Value: none | solid | double | dotted | dashed | dot-dash | dot-dot-dash | wave Initial: none Applies to: all elements with and generated content with textual content Inherited: no Percentages: N/A Media: visual Computed value: specified value (except for initial and inherit)

Please, do not add useless advocacy comments to a bug report. Use the forums instead.

Spec's been updated, see http://dev.w3.org/csswg/css3-text/#text-decoration-style

(In reply to comment #12) > Spec's been updated, see > http://dev.w3.org/csswg/css3-text/#text-decoration-style now, we can implement the solid/dotted/dashed/dot-dash/dot-dot-dash styles easily. double: we need to think about overflowed decoration line rendering. wave: we need to think about overflow too, and we need to find good way for wave line rendering.

I fixed bug 392785, so, we can implement this easily. However, I think that it is pretty late for Gecko1.9. So, I will take this bug after Gecko1.9.

taking. but we should fix bug 403524 before this.

I have a worry. The text-decoration-width is gone on the latest draft (comment 12). That is very good news for us. Because that is too difficult work for overflow issues. However, we still have the overflow issue on 'wavy' style and 'double' style. If such style is specified, we have a performance issue like bug 421353. a { text-decoration: none; } a:hover { text-decoration: underline wavy; } Then, only hovered style may need overflow area for wavy line. But for computing it, we need to rise reflow. But that is very bad... roc or dbaron, do you have good idea for this issue? # If the authors specify the following style, we don't need to recompute the overflow area, but that is not good for us... a { text-decoration: none wavy; } a:hover { text-decoration: underline wavy; } or a { text-decoration: none wavy; } a:hover { text-decoration-line: underline; }

We can special-case the use of wavy and double and do reflows for them.

(In reply to comment #17) > We can special-case the use of wavy and double and do reflows for them. Excuse me, I cannot understand what you meant.

Changes to and from "wavy" and "double" can trigger reflow hints and be slow.

(In reply to comment #19) > Changes to and from "wavy" and "double" can trigger reflow hints and be slow. ah, ok.

fantasai: Hi, the latest WD of comment 12 doesn't define the blink value of text-decoration shorthand property. Isn't it still supported only with text-decoration property? And also dot-dash, dot-dot-dash and wave are dropped from border-style. I think that dot-dash and dot-dot-dash are not needed. But wave is needed only for text-decoration-style. How about you? dbaron: It seems that https://developer.mozilla.org/En/Adding_a_new_style_property is old. Do you have a plan to update it? And we should use -moz- prefix for the related new properties, but can we change text-decoration property behavior without the prefix? It seems bad. Should I add -moz-text-decoration property for the new behavior of CSS3?

Wrt spec: Fixed. :) Wrt -moz- and the text-decoration shorthand: how about leaving the color and style out of the shorthand for now, just require the longhand properties if you want to set those aspects?

I need to learn about ":visited". Where are the documents about that?

http://dbaron.org/mozilla/visited-privacy , perhaps? Hopefully this won't conflict too much with bug 403524 (which we *might* have to do for Firefox 4).

(In reply to comment #27) > http://dbaron.org/mozilla/visited-privacy , perhaps? Thank you. > Hopefully this won't conflict too much with bug 403524 (which we *might* have > to do for Firefox 4). Part2 and Part3 may be conflict, but I think it's easy to merge. And I think that this should be landed after Fx4 is branched. If we'll land this for Fx4, we need to fix bug 537230 for quirks mode rendering too.

test builds: http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/masayuki@d-toybox.com-e41111cad7bf/

Ready to review, but we should wait until branching for Gecko 2.0.

Comment on attachment 521181 [details] [diff] [review] Part1. Adding -moz-text-decoration-color and -moz-text-decoration-style. v1.3 Part1 is outdated and I found some bugs. I'll post new patch.

updated for latest trunk.

Comment on attachment 521845 [details] [diff] [review] Part1. Adding -moz-text-decoration-color and -moz-text-decoration-style. v1.5 the spec actually changed wave back to wavy. So you should search-and-replace the patch for wave->wavy and WAVE->WAVY You'll need to update to trunk again now that bug 645620 landed, but the update is really easy: you don't need the nsCSSStruct.h changes anymore, and you have to remove two of the lines from each CSS_PROP_TEXTRESET entry in nsCSSPropList.h (see patch 5 in that bug). Also, in the nsCSSPropList.h changes, please don't use _moz_ prefixes on the second parameter. (Some properties have them, some don't -- and we're really better off leaving them out.) That'll also require some search-replacing. In the third parameter in the nsCSSPropList.h changes, you should use the CSS_PROP_DOMPROP_PREFIXED() macro instead of writing the "Moz". This means that ValueForMozTextDecorationColor() will change to ValueForTextDecorationColor(), etc. (in nsRuleNode.cpp). Also, this comment: >+ VARIANT_HCK, // used only internally is incorrect since you use CSS_PROPERTY_PARSE_VALUE. Just remove the comment. >+#define NS_STYLE_TEXT_DECORATION_STYLE_NONE 0 // not in CSS spec, but useful for selection underline Are you using it? If not, please don't add this; the spec has the text-decoration-line property that does this and more. It's also bad that you have a value that we don't know how to serialize (e.g., in GetComputedStyle). So actually I think you should just remove it. In property_database.js, you should also test -moz-use-text-color (in the initial_values line). nsRuleNode.cpp: >+ else if (eCSSUnit_Enumerated == decorationColorValue->GetUnit()) { >+ switch (decorationColorValue->GetIntValue()) { >+ case NS_STYLE_COLOR_MOZ_USE_TEXT_COLOR: >+ text->SetDecorationColorToForeground(); >+ break; >+ } >+ } >+ else if (eCSSUnit_Initial == decorationColorValue->GetUnit()) { >+ text->SetDecorationColorToForeground(); >+ } It's better to combine these two cases, as: + else if (eCSSUnit_Initial == decorationColorValue->GetUnit() || + eCSSUnit_Enumerated == decorationColorValue->GetUnit()) { + NS_ABORT_IF_FALSE(eCSSUnit_Enumerated != decorationColorValue->GetUnit() || + decorationColorValue->GetIntValue() == + NS_STYLE_COLOR_MOZ_USE_TEXT_COLOR, + "unexpected enumerated value"); + text->SetDecorationColorToForeground(); + } nsStyleContext.cpp: >+ if (thisVisDecColorIsFG != otherVisDecColorIsFG || >+ (!thisVisDecColorIsFG && thisVisDecColor != otherVisDecColorIsFG)) { The second |otherVisDecColorIsFG| should be |otherVisDecColor|. nsStyleStruct.h: >+ void SetDecorationStyle(PRUint8 aStyle) >+ { >+ mTextDecorationStyle &= ~BORDER_STYLE_MASK; >+ mTextDecorationStyle |= (aStyle & BORDER_STYLE_MASK); >+ } This should have an NS_ABORT_IF_FALSE((aStyle & BORDER_STYLE_MASK) == aStyle, "style doesn't fit"); r=dbaron with those things fixed

(In reply to comment #41) > Comment on attachment 521845 [details] [diff] [review] > Part1. Adding -moz-text-decoration-color and -moz-text-decoration-style. v1.5 > > >+#define NS_STYLE_TEXT_DECORATION_STYLE_NONE 0 // not in CSS spec, but useful for selection underline > > Are you using it? If not, please don't add this; the spec has > the text-decoration-line property that does this and more. > > It's also bad that you have a value that we don't know how to serialize > (e.g., in GetComputedStyle). > > So actually I think you should just remove it. Hmm, we've used the decoration-style-none already (see part 2). If a clause in IME composition string doesn't need underline, it needs to indicate the none style. > > > In property_database.js, you should also test -moz-use-text-color (in > the initial_values line). > > nsRuleNode.cpp: > > >+ else if (eCSSUnit_Enumerated == decorationColorValue->GetUnit()) { > >+ switch (decorationColorValue->GetIntValue()) { > >+ case NS_STYLE_COLOR_MOZ_USE_TEXT_COLOR: > >+ text->SetDecorationColorToForeground(); > >+ break; > >+ } > >+ } > >+ else if (eCSSUnit_Initial == decorationColorValue->GetUnit()) { > >+ text->SetDecorationColorToForeground(); > >+ } > > It's better to combine these two cases, as: > > + else if (eCSSUnit_Initial == decorationColorValue->GetUnit() || > + eCSSUnit_Enumerated == decorationColorValue->GetUnit()) { > + NS_ABORT_IF_FALSE(eCSSUnit_Enumerated != decorationColorValue->GetUnit() > || > + decorationColorValue->GetIntValue() == > + NS_STYLE_COLOR_MOZ_USE_TEXT_COLOR, > + "unexpected enumerated value"); > + text->SetDecorationColorToForeground(); > + } > > nsStyleContext.cpp: > >+ if (thisVisDecColorIsFG != otherVisDecColorIsFG || > >+ (!thisVisDecColorIsFG && thisVisDecColor != otherVisDecColorIsFG)) { > > The second |otherVisDecColorIsFG| should be |otherVisDecColor|. > > nsStyleStruct.h: > > >+ void SetDecorationStyle(PRUint8 aStyle) > >+ { > >+ mTextDecorationStyle &= ~BORDER_STYLE_MASK; > >+ mTextDecorationStyle |= (aStyle & BORDER_STYLE_MASK); > >+ } > > This should have an > NS_ABORT_IF_FALSE((aStyle & BORDER_STYLE_MASK) == aStyle, > "style doesn't fit"); > > r=dbaron with those things fixed

Comment on attachment 521183 [details] [diff] [review] Part2. Cleaning up current text decoration implementation v1.0 Change WAVE back to WAVY, and either remove NONE or (if you have to) add a -moz-none value to the CSS property so we have a way to serialize the computed value, and r=dbaron.

(In reply to comment #42) > Hmm, we've used the decoration-style-none already (see part 2). If a clause in > IME composition string doesn't need underline, it needs to indicate the none > style. Then the property should accept a -moz-none value so that GetComputedStyle() doesn't break when it's the computed value.

Comment on attachment 522895 [details] [diff] [review] Part1. Adding -moz-text-decoration-color and -moz-text-decoration-style. v1.6 (mq) Boris, would you sr the interface change?

Comment on attachment 522895 [details] [diff] [review] Part1. Adding -moz-text-decoration-color and -moz-text-decoration-style. v1.6 (mq) sr=me, though it might be good to add these at the end of the Moz list.

Thank you, Boris.

Comment on attachment 521185 [details] [diff] [review] Part3. Implement text-decoration-color and text-decoration-style rendering. v1.2 In nsHTMLContainerFrame::GetTextDecorations: >- aDecorations = this->GetStyleTextReset()->mTextDecoration & >+ const nsStyleTextReset* styleTextReset = this->GetStyleTextReset(); >+ aDecorations = styleTextReset->mTextDecoration & > NS_STYLE_TEXT_DECORATION_LINES_MASK; > if (aDecorations) { >- nscolor color = this->GetVisitedDependentColor(eCSSProperty_color); >- aUnderColor = color; >- aOverColor = color; >- aStrikeColor = color; + nscolor color = >+ this->GetVisitedDependentColor(eCSSProperty__moz_text_decoration_color); >+ aUnderColor = aOverColor = aStrikeColor = color; >+ aUnderStyle = aOverStyle = aStrikeStyle = >+ this->GetStyleTextReset()->GetDecorationStyle(); On the last line quoted, use styleTextReset instead of this->GetStyleTextReset(). In nsHTMLContainerFrame.h: >+ * @param aStyle the style of the text-decoration You should mention here that it's one of the NS_STYLE_TEXT_DECORATION_STYLE_* constants. Probably the same for the previous function as well. In nsTextBoxFrame.cpp: >+ PRBool isForeground; >+ styleText->GetDecorationColor(color, isForeground); >+ if (isForeground) { >+ color = context->GetStyleColor()->mColor; >+ } This should actually use GetVisitedDependentColor. That it wasn't before was a bug -- but you should fix that. r=dbaron with that

Comment on attachment 522898 [details] [diff] [review] Part4. reftests v1.1 I don't think it's worth my trying to review these in detail -- I'll just rubber-stamp them. r=dbaron But you should probably avoid the "\ No newline at end of file" by adding a newline at the end of those files (just one though -- no need for a blank line to show up).

(That said, I'm happy about the use of != tests in addition to == tests.)

http://hg.mozilla.org/mozilla-central/rev/003589ae679e http://hg.mozilla.org/mozilla-central/rev/a0a53ca7935d http://hg.mozilla.org/mozilla-central/rev/94095903e2ca http://hg.mozilla.org/mozilla-central/rev/ca40b4da1f5a

This is awesome, albeit 11 years... Just to be clear, the target milestone 2.2 means gecko 2.2? Cheers - jon

Yup, that's correct.

I'm looking forward for this to be in the nightlies and writing dev doc for it. Test uri: http://jsfiddle.net/dotnetCarpenter/LEkNZ/4/embedded/result/ ps. use nightly ff (as of writing - not working)

The points about current implementation are: * -moz- prefix needed. * text-decoration-line hasn't been implemented yet because it's still same as text-decoration of current implementation except blink value. * text-decoration isn't shorthand property for text-decoration-* properties because text-decoration has been since CSS1 and we cannot append vender prefix for such property. * see bug 537230, there is a problem for connecting complex style lines.

So a red wave underline would be declared as: text-decoration: underline; -moz-text-decoration-style: wave; -moz-text-decoration-color: red; http://jsfiddle.net/dotnetCarpenter/LEkNZ/6/ I've tested with the 31th of Marts nightly and can't get it to work.

The color works fine here, and the style as well once you change wave to wavy.

Sorry my bad... hmm on second glance at the specs, they specify wave. Was this changes based on discussions on the w3 mailing list? Would think that the draft paper was up to date. Either way, I changed it to wavy and it works. Yay! http://jsfiddle.net/dotnetCarpenter/LEkNZ/11/

The Editor's draft changed to wavy: http://dev.w3.org/csswg/css3-text/#text-decoration-style0

Masayuki, I think you should implement text-decoration as the shorthand per spec. It might not accept the new values, but the behavior it has of resetting the color and the style is significant.

Ah, sounds reasonable. How about you, dbaron?

Yes, sounds correct. Sorry, I should have caught that in review. We should add -moz-text-decoration-line to be what text-decoration is now, and make text-decoration a shorthand. Except I'm not quite sure how to do that if blink isn't a value of -moz-text-decoration-line -- we'd need a place to store it, and I'd rather have it be a value of -moz-text-decoration-line than have a fourth, hidden property there. You don't need to do cancel-* yet.

filed bug 647421.

(In reply to comment #58) > This is awesome, albeit 11 years... That's okay, I didn't need it urgently. ;-) Thanks, Masayuki!

I'm inclined to back this out for Firefox 5 since we haven't yet fixed bug 647421.

(In reply to comment #71) > I'm inclined to back this out for Firefox 5 since we haven't yet fixed bug > 647421. Yeah. I agree. We don't have much time for Fx5 due to the irregular schedule.

This should be backed out on aurora tomorrow.

bug 649551 covers backing this out from Aurora.

Documented: https://developer.mozilla.org/en/CSS/text-decoration-style https://developer.mozilla.org/en/CSS/text-decoration-color And added to Firefox 6 for developers.