-
Notifications
You must be signed in to change notification settings - Fork 2.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
update definition of fontBoundingBoxAscent/Descent and emHeightAscent… #5429
Conversation
Could you clarify why this is only changing two instances of "line box"? Are the others still correct? If so, I think we should have a note explaining the situation as the resulting section ends up being a bit confusing. |
You are right, I have updated all line box to inline box in TextMetrics. Thank you. |
…/Descent fix compile address comments address comments address comments a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So after this there are two remaining instances of line box in a different section. Are those still correct? Do they need a follow-up issue?
I have created a new issue to CSS for following up this conversation: #5656 Updated the note as requested. |
@yiyix I would expect the issue to be against https://github.com/w3c/csswg-drafts. Also, since it appears you work for Google you cannot sign the Participant Agreement as an individual. |
@yiyix you need to make your (assumed) membership of https://github.com/googlers public in order for the Participation check to be able to pass. |
@jfkthame and I discussed this and found there are concepts defined in CSS Fonts already, which we're now using. I filed w3c/csswg-drafts#5266 to ensure those concepts stick around. At this point this could use a final round of review from @yiyix / @fserb (given the many changes) and @domenic. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yiyix you need to make your (assumed) membership of https://github.com/googlers public in order for the Participation check to be able to pass.
Thank you so much for the hint, I have added myself there and set it public, Hopefully I will pass the participation check soon. Thank you so much for changing my files! I will look them in details now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Will let @annevk merge.
LGTM, Thank you for correcting my grammar errors and thank you for dig into CSS Fonts. It looks better now and i agree with the changes. |
See w3c/csswg-drafts#5312. And also #5429 for where all of this started. Ideally though w3c/csswg-drafts#5431 would be fixed too and we'd figure out how to update images/baselines.png (I've asked Ian if there's an original somewhere).
See w3c/csswg-drafts#5312. And also #5429 for where all of this started. Ideally though w3c/csswg-drafts#5431 would be fixed too and we'd figure out how to update images/baselines.png (I've asked Ian if there's an original somewhere).
See w3c/csswg-drafts#5312. And also #5429 for where all of this started. Ideally though w3c/csswg-drafts#5431 would be fixed too and we'd figure out how to update images/baselines.png (I've asked Ian if there's an original somewhere).
See w3c/csswg-drafts#5312. And also #5429 for where all of this started. This should help #6731 quite a bit too.
Following in the conclusion in #4073, I have updated the spec for fontBoundingBoxAscent/Descent and emHeightAscent/Descent.
/acknowledgements.html ( diff )
/canvas.html ( diff )
/infrastructure.html ( diff )
/references.html ( diff )