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

gfx: Map CSS `normal` font weight to Regular font weight on the Mac. #11103

Merged
merged 4 commits into from May 10, 2016

Conversation

@pcwalton
Copy link
Contributor

pcwalton commented May 9, 2016

This series of commits fixes #9487, and improves the look of nytimes.com among others.

r? @metajack


This change is Reviewable

@highfive
Copy link

highfive commented May 9, 2016

warning Warning warning

  • These commits modify gfx code, but no tests are modified. Please consider adding a test!
@pcwalton
Copy link
Contributor Author

pcwalton commented May 9, 2016

@pcwalton pcwalton force-pushed the pcwalton:mac-font-matching branch from 7137626 to 8d74e7d May 10, 2016
@highfive
Copy link

highfive commented May 10, 2016

New code was committed to pull request.

@metajack
Copy link
Contributor

metajack commented May 10, 2016

Reviewed 2 of 2 files at r1, 1 of 1 files at r2, 1 of 1 files at r3, 2 of 2 files at r4, 1 of 1 files at r5.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@metajack
Copy link
Contributor

metajack commented May 10, 2016

@bors-servo
Copy link
Contributor

bors-servo commented May 10, 2016

📌 Commit 8d74e7d has been approved by metajack

@bors-servo
Copy link
Contributor

bors-servo commented May 10, 2016

Testing commit 8d74e7d with merge e77e34c...

bors-servo added a commit that referenced this pull request May 10, 2016
gfx: Map CSS `normal` font weight to Regular font weight on the Mac.

This series of commits fixes #9487, and improves the look of nytimes.com among others.

r? @metajack

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/11103)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented May 10, 2016

💔 Test failed - mac-rel-css

@highfive
Copy link

highfive commented May 10, 2016

  ▶ PASS [expected FAIL] /css-text-3_dev/html/word-break-break-all-007.htm

  ▶ PASS [expected FAIL] /css-text-3_dev/html/word-break-normal-hi-000.htm
@highfive
Copy link

highfive commented May 10, 2016

New code was committed to pull request.

@pcwalton
Copy link
Contributor Author

pcwalton commented May 10, 2016

@bors-servo: r=metajack

Just updated test expectations.

@bors-servo
Copy link
Contributor

bors-servo commented May 10, 2016

📌 Commit 3d35e2a has been approved by metajack

@bors-servo
Copy link
Contributor

bors-servo commented May 10, 2016

Testing commit 3d35e2a with merge 9498a11...

bors-servo added a commit that referenced this pull request May 10, 2016
gfx: Map CSS `normal` font weight to Regular font weight on the Mac.

This series of commits fixes #9487, and improves the look of nytimes.com among others.

r? @metajack

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/11103)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented May 10, 2016

💔 Test failed - linux-rel

@highfive
Copy link

highfive commented May 10, 2016

  ▶ FAIL [expected PASS] /css-text-3_dev/html/word-break-normal-hi-000.htm
  └   → /css-text-3_dev/html/word-break-normal-hi-000.htm 7a83d931c4369a4ca3febd013f724b7a26fc618f
/css-text-3_dev/html/reference/word-break-normal-hi-ref-000.htm b9ee751c30ac3d59328fc32a5aca1d5b9895f8ab
Testing 7a83d931c4369a4ca3febd013f724b7a26fc618f == b9ee751c30ac3d59328fc32a5aca1d5b9895f8ab
@pcwalton pcwalton force-pushed the pcwalton:mac-font-matching branch from 3d35e2a to 6d0e4de May 10, 2016
@highfive
Copy link

highfive commented May 10, 2016

New code was committed to pull request.

@pcwalton
Copy link
Contributor Author

pcwalton commented May 10, 2016

@bors-servo: r=metajack

The failing test is Indic, so the results are meaningless.

@bors-servo
Copy link
Contributor

bors-servo commented May 10, 2016

📌 Commit 6d0e4de has been approved by metajack

found.

Partially addresses #190.
Partially addresses #9487.
pcwalton added 3 commits May 9, 2016
to 400, not 500.

CSS `normal` font-weight is specified as 400, while Mac "Regular" font
weight is reported as 0.0. On the Mac, we need to center the two ranges
on the same value to avoid choosing "Light" fonts where "Regular" would
have been more appropriate.

Closes #9487.

fix for mac
@bors-servo
Copy link
Contributor

bors-servo commented May 10, 2016

Testing commit 6d0e4de with merge 5ffc58a...

bors-servo added a commit that referenced this pull request May 10, 2016
gfx: Map CSS `normal` font weight to Regular font weight on the Mac.

This series of commits fixes #9487, and improves the look of nytimes.com among others.

r? @metajack

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/11103)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented May 10, 2016

💔 Test failed - windows

@pcwalton
Copy link
Contributor Author

pcwalton commented May 10, 2016

@bors-servo: retry

infra

@bors-servo
Copy link
Contributor

bors-servo commented May 10, 2016

Testing commit 6d0e4de with merge 1fd9c55...

bors-servo added a commit that referenced this pull request May 10, 2016
gfx: Map CSS `normal` font weight to Regular font weight on the Mac.

This series of commits fixes #9487, and improves the look of nytimes.com among others.

r? @metajack

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/11103)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented May 10, 2016

@bors-servo bors-servo merged commit 6d0e4de into servo:master May 10, 2016
3 checks passed
3 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

4 participants
You can’t perform that action at this time.