Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Make sure to hit the test font resolver in case of empty font family name. #3571

Merged
merged 1 commit into from
Apr 6, 2017

Conversation

chinmaygarde
Copy link
Member

@chinmaygarde
Copy link
Member Author

This might lead to more test failures since we are not resolving more fonts to Ahem. Earlier, fonts with an empty font family would not be resolved using our test font resolver and would instead fallback to the per platform SkFontHost.

@chinmaygarde chinmaygarde requested a review from Hixie April 6, 2017 19:25
@jason-simmons
Copy link
Member

LGTM

@chinmaygarde
Copy link
Member Author

I verified in the debugger that no instances of SkTypeface other than the Ahem font were being created. The change did however lead to the failure of a test. It could be that the test was measuring text. I am not sure. https://www.youtube.com/watch?v=n5PgWb1U5Js&feature=youtu.be

@chinmaygarde chinmaygarde merged commit 981ef6f into flutter:master Apr 6, 2017
@tvolkert
Copy link
Contributor

tvolkert commented Apr 6, 2017

This might lead to more test failures since we are not resolving more fonts to Ahem.

Yep, I saw this internally when I hard-coded a test to use Ahem. We'll probably have to update tests when we pull this engine build into flutter/flutter and into Google.

@cbracken
Copy link
Member

Heads-up: looks like this broke the paragraph overflow test:

00:29 +547 ~3 -13: /Users/cbracken/src/flutter/flutter/packages/flutter/test/rendering/paragraph_test.dart: overflow test
  Expected: <28.0>
    Actual: <14.0>

  package:test                         expect
  rendering/paragraph_test.dart 115:5  main.<fn>
  ===== asynchronous gap ===========================
  dart:async                           _StreamController.add
  websocket_impl.dart 1116             _WebSocketImpl._WebSocketImpl._fromSocket.<fn>
  dart:async                           _EventSinkWrapper.add
  websocket_impl.dart 337              _WebSocketProtocolTransformer._messageFrameEnd
  websocket_impl.dart 232              _WebSocketProtocolTransformer.add

@jason-simmons
Copy link
Member

@Hixie has a pending set of fixes to tests that depend on text metrics: flutter/flutter#9332

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

Successfully merging this pull request may close these issues.

5 participants