[SVG -> OTF Converter] Parsing failures cause use of incomplete fonts
Created attachment 268353 [details] Patch
Created attachment 268354 [details] Patch
<rdar://problem/24043104>
Comment on attachment 268354 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=268354&action=review > Source/WebCore/svg/SVGToOTFFontConversion.cpp:61 > + bool error() const Why is this function needed? Can't we check m_error in functions like SVGToOTFFontConverter::convertSVGToOTFFont() and return immediately if its value is true? > Source/WebCore/svg/SVGToOTFFontConversion.cpp:1583 > SVGToOTFFontConverter converter(element); Since SVGToOTFFontConverter::processGlyphElement() sets m_error to true and since it's called from the constructor of SVGToOTFFontConverter, should not we check converter.m_error before calling SVGToOTFFontConverter::convertSVGToOTFFont()? If m_error is true we are going to return Nullopt anyway. > Source/WebCore/svg/SVGToOTFFontConversion.cpp:1586 > + return Nullopt; Can't this if-statment be written like this? if (!converter.convertSVGToOTFFont()) return Nullopt;
Comment on attachment 268354 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=268354&action=review >> Source/WebCore/svg/SVGToOTFFontConversion.cpp:61 >> + bool error() const > > Why is this function needed? Can't we check m_error in functions like SVGToOTFFontConverter::convertSVGToOTFFont() and return immediately if its value is true? I anticipate there may be new sources of errors in the future which should result in conversion failure. >> Source/WebCore/svg/SVGToOTFFontConversion.cpp:1583 >> SVGToOTFFontConverter converter(element); > > Since SVGToOTFFontConverter::processGlyphElement() sets m_error to true and since it's called from the constructor of SVGToOTFFontConverter, should not we check converter.m_error before calling SVGToOTFFontConverter::convertSVGToOTFFont()? If m_error is true we are going to return Nullopt anyway. Yeah, this is a good idea. >> Source/WebCore/svg/SVGToOTFFontConversion.cpp:1586 >> + return Nullopt; > > Can't this if-statment be written like this? > > if (!converter.convertSVGToOTFFont()) > return Nullopt; 👍
Committed r194839: <http://trac.webkit.org/changeset/194839>