From 8e3832bacbbef4a549f64df5c978a5672e47ff2e Mon Sep 17 00:00:00 2001 From: athenian200 Date: Tue, 29 Sep 2020 11:31:46 -0500 Subject: Issue #1668 - Part 1: Implement support for caret-color property. This CSS property allows input carets (that blinking input cursor you see in text fields), to be given a custom color. This was implemented in Firefox 53, and it was such a minor feature that no one ever missed it, but I don't see any harm in implementing this. https://bugzilla.mozilla.org/show_bug.cgi?id=1063162 --- devtools/shared/css/generated/properties-db.js | 23 ++++++++++++++++++++++ layout/generic/nsFrame.cpp | 3 +-- layout/style/StyleAnimationValue.cpp | 12 ++++++++--- layout/style/StyleComplexColor.h | 21 ++++++++++++++++---- layout/style/nsCSSPropList.h | 11 +++++++++++ layout/style/nsComputedDOMStyle.cpp | 8 ++++++++ layout/style/nsComputedDOMStyle.h | 1 + layout/style/nsComputedDOMStylePropertyList.h | 1 + layout/style/nsRuleNode.cpp | 15 ++++++++++++-- layout/style/nsStyleStruct.cpp | 6 ++++++ layout/style/nsStyleStruct.h | 1 + layout/style/test/property_database.js | 12 +++++++++++ .../style/test/test_transitions_per_property.html | 15 ++++++++++++++ 13 files changed, 118 insertions(+), 11 deletions(-) diff --git a/devtools/shared/css/generated/properties-db.js b/devtools/shared/css/generated/properties-db.js index 25d9e2d334..cc17feb73f 100644 --- a/devtools/shared/css/generated/properties-db.js +++ b/devtools/shared/css/generated/properties-db.js @@ -2872,6 +2872,7 @@ exports.CSS_PROPERTIES = { "box-shadow", "box-sizing", "caption-side", + "caret-color", "clear", "clip", "clip-path", @@ -5277,6 +5278,28 @@ exports.CSS_PROPERTIES = { "unset" ] }, + "caret-color": { + "isInherited": true, + "subproperties": [ + "caret-color" + ], + "supports": [ + 2 + ], + "values": [ + "COLOR", + "auto", + "currentColor", + "hsl", + "hsla", + "inherit", + "initial", + "rgb", + "rgba", + "transparent", + "unset" + ] + }, "clear": { "isInherited": false, "subproperties": [ diff --git a/layout/generic/nsFrame.cpp b/layout/generic/nsFrame.cpp index 72923c4b7f..7f67c7d684 100644 --- a/layout/generic/nsFrame.cpp +++ b/layout/generic/nsFrame.cpp @@ -1831,8 +1831,7 @@ nsIFrame::DisplayCaret(nsDisplayListBuilder* aBuilder, nscolor nsIFrame::GetCaretColorAt(int32_t aOffset) { - // Use text color. - return StyleColor()->mColor; + return StyleColor()->CalcComplexColor(StyleUserInterface()->mCaretColor); } bool diff --git a/layout/style/StyleAnimationValue.cpp b/layout/style/StyleAnimationValue.cpp index a0f52b4ea1..d931961d40 100644 --- a/layout/style/StyleAnimationValue.cpp +++ b/layout/style/StyleAnimationValue.cpp @@ -4469,8 +4469,12 @@ StyleAnimationValue::ExtractComputedValue(nsCSSPropertyID aProperty, StyleDataAtOffset(styleStruct, ssOffset)); return true; case eStyleAnimType_ComplexColor: { - aComputedValue.SetComplexColorValue( - StyleDataAtOffset(styleStruct, ssOffset)); + auto& color = StyleDataAtOffset(styleStruct, ssOffset); + if (color.mIsAuto) { + aComputedValue.SetAutoValue(); + } else { + aComputedValue.SetComplexColorValue(color); + } return true; } case eStyleAnimType_PaintServer: { @@ -4783,7 +4787,9 @@ StyleAnimationValue::SetCurrentColorValue() void StyleAnimationValue::SetComplexColorValue(const StyleComplexColor& aColor) { - if (aColor.IsCurrentColor()) { + if (aColor.mIsAuto) { + SetAutoValue(); + } else if (aColor.IsCurrentColor()) { SetCurrentColorValue(); } else if (aColor.IsNumericColor()) { SetColorValue(aColor.mColor); diff --git a/layout/style/StyleComplexColor.h b/layout/style/StyleComplexColor.h index 4acf90a56b..6385b57a13 100644 --- a/layout/style/StyleComplexColor.h +++ b/layout/style/StyleComplexColor.h @@ -24,16 +24,29 @@ struct StyleComplexColor { nscolor mColor; uint8_t mForegroundRatio; - - static StyleComplexColor FromColor(nscolor aColor) { return {aColor, 0}; } - static StyleComplexColor CurrentColor() { return {NS_RGBA(0, 0, 0, 0), 255}; } + // Whether the complex color represents a computed-value time auto + // value. This is only a flag indicating that this value should not + // be interpolatable with other colors, while other fields still + // represents the actual used color of this value. + bool mIsAuto; + + static StyleComplexColor FromColor(nscolor aColor) { + return {aColor, 0, false}; + } + static StyleComplexColor CurrentColor() { + return {NS_RGBA(0, 0, 0, 0), 255, false}; + } + static StyleComplexColor Auto() { + return {NS_RGBA(0, 0, 0, 0), 255, true}; + } bool IsNumericColor() const { return mForegroundRatio == 0; } bool IsCurrentColor() const { return mForegroundRatio == 255; } bool operator==(const StyleComplexColor& aOther) const { return mForegroundRatio == aOther.mForegroundRatio && - (IsCurrentColor() || mColor == aOther.mColor); + (IsCurrentColor() || mColor == aOther.mColor) && + mIsAuto == aOther.mIsAuto; } bool operator!=(const StyleComplexColor& aOther) const { return !(*this == aOther); diff --git a/layout/style/nsCSSPropList.h b/layout/style/nsCSSPropList.h index 8900192454..4f79db5a55 100644 --- a/layout/style/nsCSSPropList.h +++ b/layout/style/nsCSSPropList.h @@ -1394,6 +1394,17 @@ CSS_PROP_TABLEBORDER( kCaptionSideKTable, CSS_PROP_NO_OFFSET, eStyleAnimType_Discrete) +CSS_PROP_USERINTERFACE( + caret-color, + caret_color, + CaretColor, + CSS_PROPERTY_PARSE_VALUE | + CSS_PROPERTY_IGNORED_WHEN_COLORS_DISABLED, + "", + VARIANT_AUTO | VARIANT_HC, + nullptr, + offsetof(nsStyleUserInterface, mCaretColor), + eStyleAnimType_ComplexColor) CSS_PROP_DISPLAY( clear, clear, diff --git a/layout/style/nsComputedDOMStyle.cpp b/layout/style/nsComputedDOMStyle.cpp index 1032be6516..c0cc0f84b0 100644 --- a/layout/style/nsComputedDOMStyle.cpp +++ b/layout/style/nsComputedDOMStyle.cpp @@ -4188,6 +4188,14 @@ nsComputedDOMStyle::DoGetUnicodeBidi() return val.forget(); } +already_AddRefed +nsComputedDOMStyle::DoGetCaretColor() +{ + RefPtr val = new nsROCSSPrimitiveValue; + SetValueFromComplexColor(val, StyleUserInterface()->mCaretColor); + return val.forget(); +} + already_AddRefed nsComputedDOMStyle::DoGetCursor() { diff --git a/layout/style/nsComputedDOMStyle.h b/layout/style/nsComputedDOMStyle.h index 5af518c2eb..35a6142688 100644 --- a/layout/style/nsComputedDOMStyle.h +++ b/layout/style/nsComputedDOMStyle.h @@ -488,6 +488,7 @@ private: already_AddRefed DoGetShapeOutside(); /* User interface properties */ + already_AddRefed DoGetCaretColor(); already_AddRefed DoGetCursor(); already_AddRefed DoGetForceBrokenImageIcon(); already_AddRefed DoGetIMEMode(); diff --git a/layout/style/nsComputedDOMStylePropertyList.h b/layout/style/nsComputedDOMStylePropertyList.h index 5572818105..8d4d8e45e2 100644 --- a/layout/style/nsComputedDOMStylePropertyList.h +++ b/layout/style/nsComputedDOMStylePropertyList.h @@ -101,6 +101,7 @@ COMPUTED_STYLE_PROP(box_decoration_break, BoxDecorationBreak) COMPUTED_STYLE_PROP(box_shadow, BoxShadow) COMPUTED_STYLE_PROP(box_sizing, BoxSizing) COMPUTED_STYLE_PROP(caption_side, CaptionSide) +COMPUTED_STYLE_PROP(caret_color, CaretColor) COMPUTED_STYLE_PROP(clear, Clear) COMPUTED_STYLE_PROP(clip, Clip) COMPUTED_STYLE_PROP(color, Color) diff --git a/layout/style/nsRuleNode.cpp b/layout/style/nsRuleNode.cpp index 036d97f867..66345390e5 100644 --- a/layout/style/nsRuleNode.cpp +++ b/layout/style/nsRuleNode.cpp @@ -1152,13 +1152,16 @@ SetComplexColor(const nsCSSValue& aValue, aResult = StyleComplexColor::CurrentColor(); } else if (unit == eCSSUnit_ComplexColor) { aResult = aValue.GetStyleComplexColorValue(); + } else if (unit == eCSSUnit_Auto) { + aResult = StyleComplexColor::Auto(); } else { + nscolor resultColor; if (!SetColor(aValue, aParentColor.mColor, aPresContext, - nullptr, aResult.mColor, aConditions)) { + nullptr, resultColor, aConditions)) { MOZ_ASSERT_UNREACHABLE("Unknown color value"); return; } - aResult.mForegroundRatio = 0; + aResult = StyleComplexColor::FromColor(resultColor); } } @@ -5210,6 +5213,14 @@ nsRuleNode::ComputeUserInterfaceData(void* aStartStruct, parentUI->mPointerEvents, NS_STYLE_POINTER_EVENTS_AUTO); + // caret-color: auto, color, inherit + const nsCSSValue* caretColorValue = aRuleData->ValueForCaretColor(); + SetComplexColor(*caretColorValue, + parentUI->mCaretColor, + StyleComplexColor::Auto(), + mPresContext, + ui->mCaretColor, conditions); + COMPUTE_END_INHERITED(UserInterface, ui) } diff --git a/layout/style/nsStyleStruct.cpp b/layout/style/nsStyleStruct.cpp index 3b19a44180..4c610cf085 100644 --- a/layout/style/nsStyleStruct.cpp +++ b/layout/style/nsStyleStruct.cpp @@ -4024,6 +4024,7 @@ nsStyleUserInterface::nsStyleUserInterface(StyleStructContext aContext) , mUserFocus(StyleUserFocus::None) , mPointerEvents(NS_STYLE_POINTER_EVENTS_AUTO) , mCursor(NS_STYLE_CURSOR_AUTO) + , mCaretColor(StyleComplexColor::Auto()) { MOZ_COUNT_CTOR(nsStyleUserInterface); } @@ -4035,6 +4036,7 @@ nsStyleUserInterface::nsStyleUserInterface(const nsStyleUserInterface& aSource) , mPointerEvents(aSource.mPointerEvents) , mCursor(aSource.mCursor) , mCursorImages(aSource.mCursorImages) + , mCaretColor(aSource.mCaretColor) { MOZ_COUNT_CTOR(nsStyleUserInterface); } @@ -4083,6 +4085,10 @@ nsStyleUserInterface::CalcDifference(const nsStyleUserInterface& aNewData) const hint |= nsChangeHint_NeutralChange; } + if (mCaretColor != aNewData.mCaretColor) { + hint |= nsChangeHint_RepaintFrame; + } + return hint; } diff --git a/layout/style/nsStyleStruct.h b/layout/style/nsStyleStruct.h index f49cdc43ef..d3fad30934 100644 --- a/layout/style/nsStyleStruct.h +++ b/layout/style/nsStyleStruct.h @@ -3414,6 +3414,7 @@ struct MOZ_NEEDS_MEMMOVABLE_MEMBERS nsStyleUserInterface uint8_t mCursor; // [inherited] See nsStyleConsts.h nsTArray mCursorImages; // [inherited] images and coords + mozilla::StyleComplexColor mCaretColor; // [inherited] inline uint8_t GetEffectivePointerEvents(nsIFrame* aFrame) const; }; diff --git a/layout/style/test/property_database.js b/layout/style/test/property_database.js index bc43836303..a7014c043b 100644 --- a/layout/style/test/property_database.js +++ b/layout/style/test/property_database.js @@ -2837,6 +2837,18 @@ var gCSSProperties = { other_values: [ "bottom", "left", "right", "top-outside", "bottom-outside" ], invalid_values: [] }, + "caret-color": { + domProp: "caretColor", + inherited: true, + type: CSS_TYPE_LONGHAND, + prerequisites: { "color": "black" }, + // Though "auto" is an independent computed-value time keyword value, + // it is not distinguishable from currentcolor because getComputedStyle + // always returns used value for . + initial_values: [ "auto", "currentcolor", "black", "rgb(0,0,0)" ], + other_values: [ "green", "transparent", "rgba(128,128,128,.5)", "#123" ], + invalid_values: [ "#0", "#00", "#00000", "cc00ff" ] + }, "clear": { domProp: "clear", inherited: false, diff --git a/layout/style/test/test_transitions_per_property.html b/layout/style/test/test_transitions_per_property.html index 29e2ae24c2..f188f4f6fb 100644 --- a/layout/style/test/test_transitions_per_property.html +++ b/layout/style/test/test_transitions_per_property.html @@ -1373,6 +1373,21 @@ function test_true_currentcolor_transition(prop, get_color=(x => x), is_shorthan div.style.removeProperty("color"); } +function test_auto_color_transition(prop, get_color=(x => x), is_shorthand=false) { + const msg_prefix = `color-valued property ${prop}: `; + const test_color = "rgb(51, 102, 153)"; + div.style.setProperty("transition-property", "none", ""); + div.style.setProperty(prop, "auto", ""); + let used_value_of_auto = get_color(cs.getPropertyValue(prop)); + isnot(used_value_of_auto, test_color, + msg_prefix + "ensure used auto value is different than our test color"); + + div.style.setProperty("transition-property", prop, ""); + div.style.setProperty(prop, test_color, ""); + is(get_color(cs.getPropertyValue(prop)), test_color, + msg_prefix + "not interpolatable between auto and rgb color"); +} + function get_color_from_shorthand_value(value) { var m = value.match(/rgba?\([^, ]*, [^, ]*, [^, ]*(?:, [^, ]*)?\)/); isnot(m, null, "shorthand property value should contain color"); -- cgit v1.2.3 From 1fc996152b7a1b0f61287c85ddc1698013ea63c0 Mon Sep 17 00:00:00 2001 From: athenian200 Date: Sun, 18 Oct 2020 10:12:53 -0500 Subject: Issue #1668 - Part 2: Visited color and auto support for caret-color property. Mozilla's original implementation of this failed a couple of tests, but this seems to solve all the problems. Basically, the caret-color wasn't able to be set differently based on whether a link was visited, and the auto value implementation was incomplete. The only test we fail now is the one where you have grey text on a grey background and the caret is supposed to be visible, but I think that may have been removed from the spec. Even if it wasn't, no other browser supports it anyway. --- dom/smil/nsSMILCSSProperty.cpp | 1 + layout/generic/nsFrame.cpp | 2 +- layout/style/nsRuleNode.cpp | 15 +++++++++------ layout/style/nsStyleContext.cpp | 17 ++++++++++++++++- 4 files changed, 27 insertions(+), 8 deletions(-) diff --git a/dom/smil/nsSMILCSSProperty.cpp b/dom/smil/nsSMILCSSProperty.cpp index 14e428c05b..070f3489ed 100644 --- a/dom/smil/nsSMILCSSProperty.cpp +++ b/dom/smil/nsSMILCSSProperty.cpp @@ -195,6 +195,7 @@ nsSMILCSSProperty::IsPropertyAnimatable(nsCSSPropertyID aPropID) // writing-mode switch (aPropID) { + case eCSSProperty_caret_color: case eCSSProperty_clip: case eCSSProperty_clip_rule: case eCSSProperty_clip_path: diff --git a/layout/generic/nsFrame.cpp b/layout/generic/nsFrame.cpp index 7f67c7d684..a9e6354abe 100644 --- a/layout/generic/nsFrame.cpp +++ b/layout/generic/nsFrame.cpp @@ -1831,7 +1831,7 @@ nsIFrame::DisplayCaret(nsDisplayListBuilder* aBuilder, nscolor nsIFrame::GetCaretColorAt(int32_t aOffset) { - return StyleColor()->CalcComplexColor(StyleUserInterface()->mCaretColor); + return nsLayoutUtils::GetColor(this, eCSSProperty_caret_color); } bool diff --git a/layout/style/nsRuleNode.cpp b/layout/style/nsRuleNode.cpp index 66345390e5..1136edee22 100644 --- a/layout/style/nsRuleNode.cpp +++ b/layout/style/nsRuleNode.cpp @@ -5142,6 +5142,13 @@ nsRuleNode::ComputeUserInterfaceData(void* aStartStruct, { COMPUTE_START_INHERITED(UserInterface, ui, parentUI) + auto setComplexColor = [&](const nsCSSValue* aValue, + StyleComplexColor nsStyleUserInterface::* aField) { + SetComplexColor(*aValue, parentUI->*aField, + StyleComplexColor::Auto(), + mPresContext, ui->*aField, conditions); + }; + // cursor: enum, url, inherit const nsCSSValue* cursorValue = aRuleData->ValueForCursor(); nsCSSUnit cursorUnit = cursorValue->GetUnit(); @@ -5214,12 +5221,8 @@ nsRuleNode::ComputeUserInterfaceData(void* aStartStruct, NS_STYLE_POINTER_EVENTS_AUTO); // caret-color: auto, color, inherit - const nsCSSValue* caretColorValue = aRuleData->ValueForCaretColor(); - SetComplexColor(*caretColorValue, - parentUI->mCaretColor, - StyleComplexColor::Auto(), - mPresContext, - ui->mCaretColor, conditions); + setComplexColor(aRuleData->ValueForCaretColor(), + &nsStyleUserInterface::mCaretColor); COMPUTE_END_INHERITED(UserInterface, ui) } diff --git a/layout/style/nsStyleContext.cpp b/layout/style/nsStyleContext.cpp index 4b1a14897a..38b422bd73 100644 --- a/layout/style/nsStyleContext.cpp +++ b/layout/style/nsStyleContext.cpp @@ -1255,6 +1255,17 @@ nsStyleContext::CalcStyleDifferenceInternal(StyleContextLike* aNewContext, } } + // NB: Calling Peek on |this|, not |thisVis| (see above). + if (!change && PeekStyleUserInterface()) { + const nsStyleUserInterface *thisVisUserInterface = thisVis->StyleUserInterface(); + const nsStyleUserInterface *otherVisUserInterface = otherVis->StyleUserInterface(); + if (thisVisUserInterface->mCaretColor != + otherVisUserInterface->mCaretColor) { + change = true; + } + } + + if (change) { hint |= nsChangeHint_RepaintFrame; } @@ -1487,6 +1498,9 @@ ExtractColor(nsCSSPropertyID aProperty, case StyleAnimationValue::eUnit_ComplexColor: return Some(aStyleContext->StyleColor()-> CalcComplexColor(val.GetStyleComplexColorValue())); + case StyleAnimationValue::eUnit_Auto: + return Some(aStyleContext->StyleColor()-> + CalcComplexColor(StyleComplexColor::Auto())); default: return Nothing(); } @@ -1508,7 +1522,8 @@ static const ColorIndexSet gVisitedIndices[2] = { { 0, 0 }, { 1, 0 } }; nscolor nsStyleContext::GetVisitedDependentColor(nsCSSPropertyID aProperty) { - NS_ASSERTION(aProperty == eCSSProperty_color || + NS_ASSERTION(aProperty == eCSSProperty_caret_color || + aProperty == eCSSProperty_color || aProperty == eCSSProperty_background_color || aProperty == eCSSProperty_border_top_color || aProperty == eCSSProperty_border_right_color || -- cgit v1.2.3