Skip to content

Commit

Permalink
Wait until doc-ready before scheduling font-wait. (#10279)
Browse files Browse the repository at this point in the history
The 3s timeout counts from the time the font is requested which cannot be done before the doc is ready to be styled.

Follow up for #10195 and #10267
  • Loading branch information
cramforce authored Jul 6, 2017
1 parent 736045c commit 606350c
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 29 deletions.
30 changes: 15 additions & 15 deletions src/service/resources-impl.js
Original file line number Diff line number Diff line change
Expand Up @@ -257,22 +257,22 @@ export class Resources {
}
this.schedulePass();
this.monitorInput_();
});

// Safari 10 and under incorrectly estimates font spacing for `@font-face`
// fonts. This leads to wild measurement errors. The best course of action
// is to remeasure everything on window.onload or font timeout (3s),
// whichever is earlier. This has to be done on the global window because
// this is where the fonts are always added. Unfortunately,
// `document.fonts.ready` cannot be used here due to
// https://bugs.webkit.org/show_bug.cgi?id=174030.
// See https://bugs.webkit.org/show_bug.cgi?id=174031 for more details.
Promise.race([
loadPromise(this.win),
timerFor(this.win).promise(3100),
]).then(() => {
this.relayoutAll_ = true;
this.schedulePass();
// Safari 10 and under incorrectly estimates font spacing for
// `@font-face` fonts. This leads to wild measurement errors. The best
// course of action is to remeasure everything on window.onload or font
// timeout (3s), whichever is earlier. This has to be done on the global
// window because this is where the fonts are always added.
// Unfortunately, `document.fonts.ready` cannot be used here due to
// https://bugs.webkit.org/show_bug.cgi?id=174030.
// See https://bugs.webkit.org/show_bug.cgi?id=174031 for more details.
Promise.race([
loadPromise(this.win),
timerFor(this.win).promise(3100),
]).then(() => {
this.relayoutAll_ = true;
this.schedulePass();
});
});
}

Expand Down
37 changes: 24 additions & 13 deletions test/functional/test-resources.js
Original file line number Diff line number Diff line change
Expand Up @@ -564,25 +564,36 @@ describes.fakeWin('Resources startup', {
expect(schedulePassStub).to.not.be.called;
win.readyState = 'complete';
win.eventListeners.fire({type: 'load'});
return loadPromise(win).then(() => {
// Skip a microtask.
return Promise.race([Promise.resolve()]);
win.document.eventListeners.fire({type: 'readystatechange'});
return resources.ampdoc.whenReady().then(() => {
expect(resources.relayoutAll_).to.be.true;
// Reset to make sure it is set again on load.
resources.relayoutAll_ = false;
return loadPromise(win).then(() => {
// Skip a microtask.
return Promise.race([Promise.resolve()]);
});
}).then(() => {
expect(resources.relayoutAll_).to.be.true;
expect(schedulePassStub).to.be.calledOnce;
expect(schedulePassStub).to.be.calledTwice;
});
});

it('should run a full reload pass on fonts timeout', () => {
expect(resources.relayoutAll_).to.be.false;
expect(schedulePassStub).to.not.be.called;
clock.tick(4000);
// Skip a microtask.
return Promise.resolve().then(() => {
return Promise.race([Promise.resolve()]);
}).then(() => {
expect(resources.relayoutAll_).to.be.true;
expect(schedulePassStub).to.be.calledOnce;
win.readyState = 'complete';
win.document.eventListeners.fire({type: 'readystatechange'});
return resources.ampdoc.whenReady().then(() => {
return;
expect(resources.relayoutAll_).to.be.false;
expect(schedulePassStub).to.not.be.called;
clock.tick(4000);
// Skip a microtask.
return Promise.resolve().then(() => {
return Promise.race([Promise.resolve()]);
}).then(() => {
expect(resources.relayoutAll_).to.be.true;
expect(schedulePassStub).to.be.calledOnce;
});
});
});
});
Expand Down
5 changes: 4 additions & 1 deletion testing/fake-dom.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,10 @@ export class FakeWindow {

const spec = opt_spec || {};

/** @type {string} */
/**
* This value is reflected on this.document.readyState.
* @type {string}
*/
this.readyState = spec.readyState || 'complete';

// Passthrough.
Expand Down

0 comments on commit 606350c

Please sign in to comment.