Issue metadata
Sign in to add a comment
|
base64-encoded font isn't rendered into canvas
Reported by
sean.gil...@buzzfeed.com,
Jun 6 2018
|
||||||||||||||||||||||
Issue descriptionUserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_3) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/66.0.3359.181 Safari/537.36 Steps to reproduce the problem: 1. Download an eot font (e.g. https://www.buzzfeed.com/static/fonts/ProximaNova-Extrabold-webfont.eot) via a Promise 2. Resolve the Promise 3. Encode the font data into base64 data 4. Put the font information into a `style` block associated with a `canvas` element 5. Draw some text with this font, as an svg 6. Extract an image from the `canvas` 7. Look at the image and verify the font isn't the one you specified What is the expected behavior? The text in the canvas would be drawn in the supplied font, and the extracted image would be rendered using this data and appear in the correct font. What went wrong? It seems like this may be an error at some point during either style processing or rendering via the canvas. You can reproduce this bug here: https://codepen.io/dreki/pen/GGqzbP (if you test in Chrome 66 vs Chrome 67, you'll see that in 67, the font doesn't render correctly) Did this work before? Yes 66.0.3359.181 Chrome version: 67.0.3396.62 Channel: stable OS Version: OS X 10.13.3 Flash Version: Aside from the CodePen, I can supply you to a URL to the actual tool we've created that uses this technology. Because of this bug, our whole editorial staff may have to be moved to a different browser at least temporarily. It's pretty disruptive and it doesn't seem like our issue or something we can work around. Thank you for your time.
,
Jun 6 2018
Update: It looks like this rule broke the font rendering in 67: `font: 700 100% 118px/118px "ProximaNova Extrabold"; font-kerning: auto;` We removed `100%` and then the text rendered as expected. Here's the updated rule: `font: 700 118px/118px "ProximaNova Extrabold"; font-kerning: auto;`
,
Jun 6 2018
So it sounds like one of these may happened: - Something about the `font` rule itself changed in 67 - The code that generates this rule/generated the 'inline' version of fonts changed - Something in SVG rendering or canvas drawing changed We did notice that the generated font styles definitely did change in 67. The new style was much larger (~100KB larger), and included things like ` unicode-range: U+0-FF, U+131, U+152-153, U+2BB-2BC, U+2C6, U+2DA, U+2DC, U+2000-206F, U+2074, U+20AC, U+2122, U+2191, U+2193, U+2212, U+2215, U+FEFF, U+FFFD;`, when those weren't included before.
,
Jun 6 2018
Comment from a team member: "It's the font-stretch property (defaulting to 100%) that it's trying to insert into the font property btw (not the text-size-adjust) It's clearly indicated on MDN that you cannot have both font-weight and font-stretch in a font style, but Chrome is trying to."
,
Jun 6 2018
More conversation: "sean: so Chrome generates this? dave: Yeah this is the CSSRule magic stuff going on. I think it's coming from CSSRule.cssText ultimately."
,
Jun 6 2018
Sounds like this may have been caused by issue 827921 .
,
Jun 6 2018
I see what's going on. Per https://drafts.csswg.org/css-fonts-4/#font-prop: Values for the font-stretch property can also be included but only those supported in CSS Fonts level 3, none of the font-stretch values added in this specification can be used in the font shorthand We're serializing font-stretch percentages (defined in Fonts 4) in the font shorthand, creating a rule that cannot be reparsed.
,
Jun 7 2018
Thank you so much. We have a workaround for now, but will be very happy when this is released.
,
Jun 19 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/43f06f172201a5435d4106c9b8317e734e257aee commit 43f06f172201a5435d4106c9b8317e734e257aee Author: Chris Nardi <cnardi@chromium.org> Date: Tue Jun 19 09:51:43 2018 Serialize font-stretch values correctly in the font shorthand Per the CSS Fonts 4 spec [1], only keyword values for font-stretch are valid in the font shorthand. Our current serialization code ignores this, and outputs percentage values as well in the shorthand, meaning that the generated rule cannot be reparsed. We now check if the percentage can be converted to a keyword, and if so, output it as that keyword. Otherwise, we do not output a serialization for the font shorthand, as per the CSSOM spec [2]. [1]: https://drafts.csswg.org/css-fonts-4/#font-prop [2]: https://drafts.csswg.org/cssom/#serializing-css-values Bug: 850092 Change-Id: I7e3eec64723966b15abfa819213b95cba6cbc3d5 Reviewed-on: https://chromium-review.googlesource.com/1103856 Commit-Queue: Dominik Röttsches <drott@chromium.org> Reviewed-by: Dominik Röttsches <drott@chromium.org> Cr-Commit-Position: refs/heads/master@{#568383} [add] https://crrev.com/43f06f172201a5435d4106c9b8317e734e257aee/third_party/WebKit/LayoutTests/external/wpt/css/css-fonts/font-shorthand-serialization-font-stretch.html [modify] https://crrev.com/43f06f172201a5435d4106c9b8317e734e257aee/third_party/blink/renderer/core/css/style_property_serializer.cc [modify] https://crrev.com/43f06f172201a5435d4106c9b8317e734e257aee/third_party/blink/renderer/core/css/style_property_serializer.h
,
Jun 19 2018
I saw that y'all made this changed, reviewed it, and merged it into 'master'. Thank you so much for your very fast work on this. We feel very heard and I personally have a lot of respect for you all -- you've been very receptive and helpful in all my interactions with you.
,
Jun 19 2018
I'm glad you're happy with the speed of change, only sorry I didn't notice this bug in my original change! The fix should be in the next canary build, released tomorrow. I'm not sure that this change meets the bar for merging to M68, so the fix should be in M69. |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by dtapu...@chromium.org
, Jun 6 2018