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

Continue loading font-face sources on missing local font #12208

Merged
merged 1 commit into from Jul 6, 2016

Conversation

@nox
Copy link
Member

nox commented Jul 4, 2016

This change is Reviewable

@highfive
Copy link

highfive commented Jul 4, 2016

warning Warning warning

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

nox commented Jul 4, 2016

This should fix font issues on every website using the Google CDN for fonts, given it lists local sources first, and then url() ones.

@metajack
Copy link
Contributor

metajack commented Jul 5, 2016

@bors-servo r+


Reviewed 1 of 1 files at r1.
Review status: all files reviewed at latest revision, 1 unresolved discussion.


components/gfx/font_cache_thread.rs, line 272 [r1] (raw file):

                let templates = &mut self.web_families.get_mut(&family_name).unwrap();
                let mut found = false;
                for_each_variation(&font_face_name, |path| {

Was this written before iterators were a thing? Maybe file an E-Easy bug for changing this to for path in font_face_name.variants() or something.


Comments from Reviewable

@bors-servo
Copy link
Contributor

bors-servo commented Jul 5, 2016

📌 Commit 74a4d76 has been approved by metajack

@bors-servo
Copy link
Contributor

bors-servo commented Jul 5, 2016

Testing commit 74a4d76 with merge ed36fe1...

bors-servo added a commit that referenced this pull request Jul 5, 2016
Continue loading font-face sources on missing local font

<!-- 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/12208)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jul 5, 2016

💔 Test failed - linux-rel

@nox
Copy link
Member Author

nox commented Jul 5, 2016

@nox nox added S-awaiting-merge and removed S-tests-failed labels Jul 5, 2016
@bors-servo
Copy link
Contributor

bors-servo commented Jul 5, 2016

Testing commit 74a4d76 with merge fdb0683...

bors-servo added a commit that referenced this pull request Jul 5, 2016
Continue loading font-face sources on missing local font

<!-- 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/12208)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jul 5, 2016

💔 Test failed - linux-rel

@KiChjang
Copy link
Member

KiChjang commented Jul 6, 2016

@bors-servo
Copy link
Contributor

bors-servo commented Jul 6, 2016

Testing commit 74a4d76 with merge 53775a1...

bors-servo added a commit that referenced this pull request Jul 6, 2016
Continue loading font-face sources on missing local font

<!-- 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/12208)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jul 6, 2016

Testing commit 74a4d76 with merge 1642cc5...

bors-servo added a commit that referenced this pull request Jul 6, 2016
Continue loading font-face sources on missing local font

<!-- 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/12208)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jul 6, 2016

💔 Test failed - linux-rel

@nox
Copy link
Member Author

nox commented Jul 6, 2016

@nox nox added S-awaiting-merge and removed S-tests-failed labels Jul 6, 2016
@bors-servo
Copy link
Contributor

bors-servo commented Jul 6, 2016

Previous build results for android, arm32, arm64, linux-dev, mac-dev-unit, mac-rel-css, mac-rel-wpt, windows are reusable. Rebuilding only linux-rel...

@bors-servo
Copy link
Contributor

bors-servo commented Jul 6, 2016

💔 Test failed - linux-rel

@nox
Copy link
Member Author

nox commented Jul 6, 2016

@nox nox added S-awaiting-merge and removed S-tests-failed labels Jul 6, 2016
@KiChjang
Copy link
Member

KiChjang commented Jul 6, 2016

@bors-servo
Copy link
Contributor

bors-servo commented Jul 6, 2016

Testing commit 74a4d76 with merge bb1c2fe...

bors-servo added a commit that referenced this pull request Jul 6, 2016
Continue loading font-face sources on missing local font

<!-- 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/12208)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jul 6, 2016

💔 Test failed - mac-rel-wpt

@KiChjang
Copy link
Member

KiChjang commented Jul 6, 2016

@bors-servo
Copy link
Contributor

bors-servo commented Jul 6, 2016

Testing commit 74a4d76 with merge 3679b0a...

bors-servo added a commit that referenced this pull request Jul 6, 2016
Continue loading font-face sources on missing local font

<!-- 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/12208)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jul 6, 2016

@bors-servo bors-servo merged commit 74a4d76 into servo:master Jul 6, 2016
2 checks passed
2 checks passed
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.

None yet

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