From 551de121c106077a1beb3fa2f65124b6afd261eb Mon Sep 17 00:00:00 2001 From: Masayuki Nakano Date: Tue, 19 Feb 2019 06:31:28 +0000 Subject: [PATCH] Bug 998941 - part 1-7: Make HTMLEditor set InputEvent.data to serialized color value when InputEvent.inputType is "formatBackColor" or "formatForeColor" r=smaug,m_kato,emilio Although neither Chrome nor Safari does not set InputEvent.data when the event is caused by `document.execCommand()` with `backColor`, `foreColor` nor `hiliteColor`, Safari supports styling color with touchbar and in that case, Safari sets it (*1). Additionally, currently Safari uses `rgb()` to represents a color value and using same rule to serializing color value for CSS OM matches Safari's behavior and can represent any valid color values. This patch makes given color value parsed and then serialized with same code in style system. If the value is `currentcolor`, `inherit`, `initial` or `reset`, sets the value as-is for now. Additionally, when given value is invalid, sets the value as-is for forward compatibility. Note that automated tests will be added into input-events-exec-command.html by the last patch. 1. https://github.com/w3c/input-events/issues/94#issuecomment-461061517 Differential Revision: https://phabricator.services.mozilla.com/D19295 --HG-- extra : moz-landing-system : lando --- editor/libeditor/EditorBase.cpp | 36 ++++++++++++++++++++++++++++ editor/libeditor/EditorBase.h | 2 ++ editor/libeditor/HTMLStyleEditor.cpp | 16 +++++++++++++ layout/style/nsComputedDOMStyle.cpp | 18 +------------- layout/style/nsStyleUtil.cpp | 23 ++++++++++++++++++ layout/style/nsStyleUtil.h | 8 +++++++ widget/EventForwards.h | 2 ++ 7 files changed, 88 insertions(+), 17 deletions(-) diff --git a/editor/libeditor/EditorBase.cpp b/editor/libeditor/EditorBase.cpp index 94b5d62a0fc8..d78501d235e7 100644 --- a/editor/libeditor/EditorBase.cpp +++ b/editor/libeditor/EditorBase.cpp @@ -44,6 +44,7 @@ #include "mozilla/RangeBoundary.h" // for RawRangeBoundary, RangeBoundary #include "mozilla/dom/Selection.h" // for Selection, etc. #include "mozilla/Services.h" // for GetObserverService +#include "mozilla/ServoCSSParser.h" // for ServoCSSParser #include "mozilla/TextComposition.h" // for TextComposition #include "mozilla/TextInputListener.h" // for TextInputListener #include "mozilla/TextServicesDocument.h" // for TextServicesDocument @@ -103,6 +104,7 @@ #include "nsStyleConsts.h" // for NS_STYLE_DIRECTION_RTL, etc. #include "nsStyleStruct.h" // for nsStyleDisplay, nsStyleText, etc. #include "nsStyleStructFwd.h" // for nsIFrame::StyleUIReset, etc. +#include "nsStyleUtil.h" // for nsStyleUtil #include "nsTextNode.h" // for nsTextNode #include "nsThreadUtils.h" // for nsRunnable #include "prtime.h" // for PR_Now @@ -4867,4 +4869,38 @@ EditorBase::AutoEditActionDataSetter::~AutoEditActionDataSetter() { mEditorBase.mEditActionData = mParentData; } +void EditorBase::AutoEditActionDataSetter::SetColorData( + const nsAString& aData) { + if (aData.IsEmpty()) { + // When removing color/background-color, let's use empty string. + MOZ_ASSERT(!EmptyString().IsVoid()); + mData = EmptyString(); + return; + } + + bool wasCurrentColor = false; + nscolor color = NS_RGB(0, 0, 0); + if (!ServoCSSParser::ComputeColor(nullptr, NS_RGB(0, 0, 0), aData, &color, + &wasCurrentColor)) { + // If we cannot parse aData, let's set original value as-is. It could be + // new format defined by newer spec. + MOZ_ASSERT(!aData.IsVoid()); + mData = aData; + return; + } + + // If it's current color, we cannot resolve actual current color here. + // So, let's return "currentcolor" keyword, but let's use it as-is because + // there is no agreement between browser vendors. + if (wasCurrentColor) { + MOZ_ASSERT(!aData.IsVoid()); + mData = aData; + return; + } + + // Get serialized color value (i.e., "rgb()" or "rgba()"). + nsStyleUtil::GetSerializedColorValue(color, mData); + MOZ_ASSERT(!mData.IsVoid()); +} + } // namespace mozilla diff --git a/editor/libeditor/EditorBase.h b/editor/libeditor/EditorBase.h index 9f818e17aee3..4232f7ecf2ed 100644 --- a/editor/libeditor/EditorBase.h +++ b/editor/libeditor/EditorBase.h @@ -652,6 +652,8 @@ class EditorBase : public nsIEditor, void SetData(const nsAString& aData) { mData = aData; } const nsString& GetData() const { return mData; } + void SetColorData(const nsAString& aData); + void SetTopLevelEditSubAction(EditSubAction aEditSubAction, EDirection aDirection = eNone) { mTopLevelEditSubAction = aEditSubAction; diff --git a/editor/libeditor/HTMLStyleEditor.cpp b/editor/libeditor/HTMLStyleEditor.cpp index 4c9448004abb..55121e372d2d 100644 --- a/editor/libeditor/HTMLStyleEditor.cpp +++ b/editor/libeditor/HTMLStyleEditor.cpp @@ -73,6 +73,10 @@ nsresult HTMLEditor::SetInlinePropertyAsAction(nsAtom& aProperty, // XXX Should we trim unnecessary whitespaces? editActionData.SetData(aValue); break; + case EditAction::eSetColorProperty: + case EditAction::eSetBackgroundColorPropertyInline: + editActionData.SetColorData(aValue); + break; default: break; } @@ -120,6 +124,10 @@ HTMLEditor::SetInlineProperty(const nsAString& aProperty, // XXX Should we trim unnecessary whitespaces? editActionData.SetData(aValue); break; + case EditAction::eSetColorProperty: + case EditAction::eSetBackgroundColorPropertyInline: + editActionData.SetColorData(aValue); + break; default: break; } @@ -1246,6 +1254,10 @@ nsresult HTMLEditor::RemoveInlinePropertyAsAction(nsAtom& aProperty, MOZ_ASSERT(!EmptyString().IsVoid()); editActionData.SetData(EmptyString()); break; + case EditAction::eRemoveColorProperty: + case EditAction::eRemoveBackgroundColorPropertyInline: + editActionData.SetColorData(EmptyString()); + break; default: break; } @@ -1273,6 +1285,10 @@ HTMLEditor::RemoveInlineProperty(const nsAString& aProperty, MOZ_ASSERT(!EmptyString().IsVoid()); editActionData.SetData(EmptyString()); break; + case EditAction::eRemoveColorProperty: + case EditAction::eRemoveBackgroundColorPropertyInline: + editActionData.SetColorData(EmptyString()); + break; default: break; } diff --git a/layout/style/nsComputedDOMStyle.cpp b/layout/style/nsComputedDOMStyle.cpp index 7aa04e91fa03..1e7fae8fe15b 100644 --- a/layout/style/nsComputedDOMStyle.cpp +++ b/layout/style/nsComputedDOMStyle.cpp @@ -1021,23 +1021,7 @@ already_AddRefed nsComputedDOMStyle::DoGetBottom() { /* static */ void nsComputedDOMStyle::SetToRGBAColor( nsROCSSPrimitiveValue* aValue, nscolor aColor) { nsAutoString string; - const bool hasAlpha = NS_GET_A(aColor) != 255; - if (hasAlpha) { - string.AppendLiteral("rgba("); - } else { - string.AppendLiteral("rgb("); - } - string.AppendInt(NS_GET_R(aColor)); - string.AppendLiteral(", "); - string.AppendInt(NS_GET_G(aColor)); - string.AppendLiteral(", "); - string.AppendInt(NS_GET_B(aColor)); - if (hasAlpha) { - string.AppendLiteral(", "); - float alpha = nsStyleUtil::ColorComponentToFloat(NS_GET_A(aColor)); - nsStyleUtil::AppendCSSNumber(alpha, string); - } - string.AppendLiteral(")"); + nsStyleUtil::GetSerializedColorValue(aColor, string); aValue->SetString(string); } diff --git a/layout/style/nsStyleUtil.cpp b/layout/style/nsStyleUtil.cpp index 9ea38748f4d0..1d1cfe864bb8 100644 --- a/layout/style/nsStyleUtil.cpp +++ b/layout/style/nsStyleUtil.cpp @@ -269,6 +269,29 @@ void nsStyleUtil::AppendEscapedCSSString(const nsAString& aString, return rounded; } +/* static */ void nsStyleUtil::GetSerializedColorValue( + nscolor aColor, nsAString& aSerializedColor) { + MOZ_ASSERT(aSerializedColor.IsEmpty()); + + const bool hasAlpha = NS_GET_A(aColor) != 255; + if (hasAlpha) { + aSerializedColor.AppendLiteral("rgba("); + } else { + aSerializedColor.AppendLiteral("rgb("); + } + aSerializedColor.AppendInt(NS_GET_R(aColor)); + aSerializedColor.AppendLiteral(", "); + aSerializedColor.AppendInt(NS_GET_G(aColor)); + aSerializedColor.AppendLiteral(", "); + aSerializedColor.AppendInt(NS_GET_B(aColor)); + if (hasAlpha) { + aSerializedColor.AppendLiteral(", "); + float alpha = nsStyleUtil::ColorComponentToFloat(NS_GET_A(aColor)); + nsStyleUtil::AppendCSSNumber(alpha, aSerializedColor); + } + aSerializedColor.AppendLiteral(")"); +} + /* static */ bool nsStyleUtil::IsSignificantChild( nsIContent* aChild, bool aWhitespaceIsSignificant) { bool isText = aChild->IsText(); diff --git a/layout/style/nsStyleUtil.h b/layout/style/nsStyleUtil.h index 9d670a90e3be..25e6a470e983 100644 --- a/layout/style/nsStyleUtil.h +++ b/layout/style/nsStyleUtil.h @@ -90,6 +90,14 @@ class nsStyleUtil { */ static float ColorComponentToFloat(uint8_t aAlpha); + /** + * GetSerializedColorValue() computes serialized color value of aColor and + * returns it with aSerializedColor. + * https://drafts.csswg.org/cssom/#serialize-a-css-component-value + */ + static void GetSerializedColorValue(nscolor aColor, + nsAString& aSerializedColor); + /* * Does this child count as significant for selector matching? */ diff --git a/widget/EventForwards.h b/widget/EventForwards.h index f60d84f6b907..50f055dc2726 100644 --- a/widget/EventForwards.h +++ b/widget/EventForwards.h @@ -164,6 +164,8 @@ inline bool IsDataAvailableOnHTMLEditor(EditorInputType aInputType) { case EditorInputType::eFormatSetBlockTextDirection: case EditorInputType::eFormatSetInlineTextDirection: case EditorInputType::eInsertLink: + case EditorInputType::eFormatBackColor: + case EditorInputType::eFormatFontColor: case EditorInputType::eFormatFontName: return true; default: