Closed Bug 648299 Opened 14 years ago Closed 14 years ago

-moz-text-decoration-style: -moz-none; does paint the decoration lines

Categories

(Core :: Layout: Text and Fonts, defect)

defect
Not set
minor

Tracking

()

RESOLVED FIXED
mozilla6

People

(Reporter: masayuki, Assigned: masayuki)

References

Details

(Keywords: dev-doc-complete)

Attachments

(1 file)

Attached patch Patch v1.0 (deleted) — Splinter Review
I missed to implement the actual behavior of -moz-none in bug 59109. I realized this bug at working on bug 537230...
Attachment #524417 - Flags: review?(dbaron)
Status: NEW → ASSIGNED
Keywords: dev-doc-needed
> dev-doc-needed I think that we shouldn't document -moz-none for -moz-text-decoration-style. The value is used only internally, and web developers shouldn't use it. Furthermore, we're not going to propose it for web standards.
Documentation is also required for internal use. Generally everything should be documented (IMHO), where appropriate with a hint like "not intended for use in public web pages". We have such things for some other CSS properties or values.
Comment on attachment 524417 [details] [diff] [review] Patch v1.0 >- NS_ASSERTION(aStyle != NS_STYLE_TEXT_DECORATION_STYLE_NONE, "aStyle is none"); >+ NS_ENSURE_TRUE(aStyle != NS_STYLE_TEXT_DECORATION_STYLE_NONE, ); This should either stay an assertion or that it be an if (...) { return; }. If you don't actually hit it, leave it an assertion, otherwise make it a return without any console output. >- if (decorations == NS_STYLE_TEXT_DECORATION_NONE) >+ if (decorations == NS_STYLE_TEXT_DECORATION_NONE || >+ (underStyle == NS_STYLE_TEXT_DECORATION_STYLE_NONE && >+ overStyle == NS_STYLE_TEXT_DECORATION_STYLE_NONE && >+ strikeStyle == NS_STYLE_TEXT_DECORATION_STYLE_NONE)) { > return NS_OK; >+ } I don't think this check is worth the extra code, and it also looks like it will read uninitialized memory and therefore not actually be useful unless all three decorations are present and all set to a none style. I'd say just leave this as it was. r=dbaron with that
Attachment #524417 - Flags: review?(dbaron) → review+
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla6
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: