Created attachment 104023 [details] Repro Repro: <script> oContext2d=document.getCSSCanvasContext("2d","",0); oContext2d.font="small-caps.0ex G"; </script> double CSSPrimitiveValue::computeLengthDouble(RenderStyle* style, RenderStyle* rootStyle, double multiplier, bool computingFontSize) { <<<snip>>> switch (type) { <<<snip>>> case CSS_EXS: // FIXME: We have a bug right now where the zoom will be applied twice to EX units. // We really need to compute EX using fontMetrics for the original specifiedSize and not use // our actual constructed rendering font. applyZoomMultiplier = false; factor = style->fontMetrics().xHeight(); break; There is no font list (yet), leading to an ASSERT and NULL ptr here: inline const SimpleFontData* Font::primaryFont() const { ASSERT(m_fontList); return m_fontList->primarySimpleFontData(this); } id: chrome.dll!WebCore::RenderStyle::fontMetrics ReadAV@NULL (73a53e2ce303ee898f439c13af2ceffd) description: Attempt to read from unallocated NULL pointer+0x30 in chrome.dll!WebCore::RenderStyle::fontMetrics stack: chrome.dll!WebCore::RenderStyle::fontMetrics chrome.dll!WebCore::CSSPrimitiveValue::computeLengthDouble chrome.dll!WebCore::CSSPrimitiveValue::computeLength<...> chrome.dll!WebCore::CSSStyleSelector::applyProperty chrome.dll!WebCore::CSSStyleSelector::applyProperty chrome.dll!WebCore::CSSStyleSelector::applyPropertyToStyle chrome.dll!WebCore::CanvasRenderingContext2D::setFont chrome.dll!WebCore::CanvasRenderingContext2DInternal::fontAttrSetter chrome.dll!v8::internal::JSObject::SetPropertyWithCallback chrome.dll!v8::internal::JSObject::SetPropertyForResult chrome.dll!v8::internal::JSReceiver::SetProperty chrome.dll!v8::internal::StoreIC::Store chrome.dll!v8::internal::StoreIC_Miss ...
Chromium https://code.google.com/p/chromium/issues/detail?id=93035
<rdar://problem/9970343>
Created attachment 108695 [details] Proposed work-around: Add a manual update.
Comment on attachment 108695 [details] Proposed work-around: Add a manual update. View in context: https://bugs.webkit.org/attachment.cgi?id=108695&action=review > Source/WebCore/css/CSSStyleSelector.cpp:2987 > + updateFont(); This needs a “why” comment. It’s not at all clear why you need to call this here, and not, say, between each call to applyProperty. The comment should answer that question.
Created attachment 108747 [details] Proposed work-around 2: Added a 'why' comment that should answer Darin's request.
Comment on attachment 108747 [details] Proposed work-around 2: Added a 'why' comment that should answer Darin's request. Clearing flags on attachment: 108747 Committed r96122: <http://trac.webkit.org/changeset/96122>
All reviewed patches have been landed. Closing bug.
One more thought post-landing. Now that I read the comment explaining why updateFont is needed, I am puzzled. If the font size is indeed using the "ex" unit, does that mean the font size is based on the font itself? Doesn’t the "ex" part have to be based on the font of the container or the inherited font or something? How can a font size be based on the font it’s sizing? It seems that if I specified "1ex" then there would be a sort of infinite recursion resulting in an infinitely small font.
(In reply to comment #8) > One more thought post-landing. Now that I read the comment explaining why updateFont is needed, I am puzzled. If the font size is indeed using the "ex" unit, does that mean the font size is based on the font itself? That's what is happening here. That's because the applyPropertyToStyle sets parentStyle to the current style. > Doesn’t the "ex" part have to be based on the font of the container or the inherited font or something? Normally you would use your parent's font size (which is what CSS defines). For canvas, the spec does not say whether we should use the parent's style to resolve any ambiguous properties like this case. > How can a font size be based on the font it’s sizing? People using 'ex' in this case likely have the wrong expectations. However nothing prevents CanvasRenderingContext from adding this circular dependency. > It seems that if I specified "1ex" then there would be a sort of infinite recursion resulting in an infinitely small font. This will actually not happen. We restart from a clean style every time you set the font on the CanvasRenderingContext. Also the style application in this case would resolve the font size once.
(In reply to comment #0) > <script> > oContext2d=document.getCSSCanvasContext("2d","",0); > oContext2d.font="small-caps.0ex G"; > </script> This tests fails on all platform with: "CONSOLE MESSAGE: line 8: TypeError: Not enough arguments" getCSSCanvasContext() has 4 parameters, and you passed only 3 parameters.
I tried to add 0 as a 4. parameter and the test pass for me: - oContext2d=document.getCSSCanvasContext("2d","",0); + oContext2d=document.getCSSCanvasContext("2d","",0,0);
Patch landed with this change in r96204. http://trac.webkit.org/changeset/96204
Could you guys confirm if our fix is correct or not?
(In reply to comment #13) > Could you guys confirm if our fix is correct or not? Thanks, that's a good fix. The important lines are the one setting the font on the CanvasGraphicsContext2D.