Skip to content
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

Fix text actualBoundingBoxLeft and actualBoundingBoxRight measures #1776

Merged
merged 1 commit into from
Apr 20, 2021

Conversation

mserege
Copy link

@mserege mserege commented Mar 30, 2021

Fix text actualBoundingBoxLeft and actualBoundingBoxRight measures by using ink_rect instead of logical_rect

Issue : #1703

  • Have you updated CHANGELOG.md?

@mserege
Copy link
Author

mserege commented Mar 30, 2021

Failed tests on Linux seems to be due to libpango package update from libpango1.0-dev_1.40.1 to libpango1.0-dev_1.44.7 as I did not touch failed tests and as tests were ok in last commit in master.

Anyone knows how to fix these failed tests?

Copy link
Collaborator

@chearon chearon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a bit frustrating that the specs don't say to use the actual glyph path extents, but the screenshots in #1703 prove that Chrome is definitely doing that, while we're using numbers from GPOS.

@zbjornson didn't this test fail during prebuilds? Are the tests using prebuilds now?

@mserege
Copy link
Author

mserege commented Apr 8, 2021

@chearon @zbjornson : any chance that this pull request can be merged soon? I

zbjornson added a commit that referenced this pull request Apr 20, 2021
@zbjornson zbjornson merged commit 234e659 into Automattic:master Apr 20, 2021
@zbjornson
Copy link
Collaborator

Are the tests using prebuilds now?

No. The tests run with ubuntu-latest, and GitHub updated that from 18 to 20 recently, which brought in a newer Pango as @mserege said.

@zbjornson didn't this test fail during prebuilds?

Ah right. I just cherry-picked the change from the prebuilds branch that loosens this test expectation into master. This commit passed after rebasing: 234e659.

Thanks @mserege!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants