Bug 1388230: Make nsColorControlFrame::UpdateColor() a no-op if value is empty (which implies our element is still being appended). r?jwatt
nsColorControlFrame::UpdateColor() looks up the color value from the
corresponding <input> element -- and it expects to receive a valid color
string, regardless of what the user/author has done (or whether they've done
anything), thanks to the HTMLInputElement sanitization code that gets run when
the value is set.
As a basic sanity-check, UpdateColor() has an assertion to verify that the
value it receives is non-empty. However, if it happens to be called while the
element is still being appended (e.g. due to greedy frame construction), then
it *can* legitimately get an empty value. So, the assertion isn't entirely valid!
Hence, this patch relaxes the assertion to only take effect after the frame has
been reflowed, and it also makes UpdateColor() a no-op in that case. This
is fine because we can expect that UpdateColor() will be called again (and will
see a non-empty color value at that point) before the frame gets
reflowed/painted.
MozReview-Commit-ID: LOymuwy6gIM
new file mode 100644
--- /dev/null
+++ b/layout/forms/crashtests/1388230-1.html
@@ -0,0 +1,3 @@
+<cite contenteditable='true'>
+ <input type='color'/>
+</cite>
new file mode 100644
--- /dev/null
+++ b/layout/forms/crashtests/1388230-2.html
@@ -0,0 +1,1 @@
+<input contenteditable='true' type='color'>
--- a/layout/forms/crashtests/crashtests.list
+++ b/layout/forms/crashtests/crashtests.list
@@ -62,8 +62,10 @@ load 959311.html
load 960277-2.html
load 997709-1.html
load 1102791.html
load 1140216.html
load 1182414.html
load 1212688.html
load 1228670.xhtml
load 1279354.html
+load 1388230-1.html
+load 1388230-2.html
--- a/layout/forms/nsColorControlFrame.cpp
+++ b/layout/forms/nsColorControlFrame.cpp
@@ -85,27 +85,41 @@ nsColorControlFrame::AppendAnonymousCont
aElements.AppendElement(mColorContent);
}
}
nsresult
nsColorControlFrame::UpdateColor()
{
// Get the color from the "value" property of our content; it will return the
- // default color (through the sanitization algorithm) if there is none.
+ // default color (through the sanitization algorithm) if the value is empty.
nsAutoString color;
HTMLInputElement* elt = HTMLInputElement::FromContent(mContent);
elt->GetValue(color, CallerType::System);
- MOZ_ASSERT(!color.IsEmpty(),
- "Content node's GetValue() should return a valid color string "
- "(the default color, in case no valid color is set)");
- // Set the background-color style property of the swatch element to this color
+ if (color.IsEmpty()) {
+ // OK, there is one case the color string might be empty -- if our content
+ // is still being created, i.e. if it has mDoneCreating==false. In that
+ // case, we simply do nothing, because we'll be called again with a complete
+ // content node before we ever reflow or paint. Specifically: we can expect
+ // that HTMLInputElement::DoneCreatingElement() will set mDoneCreating to
+ // true (which enables sanitization) and then it'll call SetValueInternal(),
+ // which produces a nonempty color (via sanitization), and then it'll call
+ // this function here, and we'll get the nonempty default color.
+ MOZ_ASSERT(HasAnyStateBits(NS_FRAME_FIRST_REFLOW),
+ "Content node's GetValue() should return a valid color string "
+ "by the time we've been reflowed (the default color, in case "
+ "no valid color is set)");
+ return NS_OK;
+ }
+
+ // Set the background-color CSS property of the swatch element to this color.
return mColorContent->SetAttr(kNameSpaceID_None, nsGkAtoms::style,
- NS_LITERAL_STRING("background-color:") + color, true);
+ NS_LITERAL_STRING("background-color:") + color,
+ /* aNotify */ true);
}
nsresult
nsColorControlFrame::AttributeChanged(int32_t aNameSpaceID,
nsIAtom* aAttribute,
int32_t aModType)
{
NS_ASSERTION(mColorContent, "The color div must exist");