New issue
Advanced search Search tips

Issue 721856 link

Starred by 2 users

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

AMP polls for styleSheets inserted with <style> due to some kind of style system bug

Project Member Reported by esprehn@chromium.org, May 12 2017

Issue description

It seems when inserting a stylesheet with <style> into the page it doesn't always apply immediately, maybe it's due to the FOUC avoidance stuff interfering? AMP is polling document.styleSheets instead to check when the sheet has actually loaded which seems bad.

The amp code is here:
https://github.com/ampproject/amphtml/blob/e00a3da988672d60fae47105c663793c1119e64a/src/style-installer.js#L58

export function installStyles(doc, cssText, cb, opt_isRuntimeCss, opt_ext) {
  const style = insertStyleElement(
      doc,
      dev().assertElement(doc.head),
      cssText,
      opt_isRuntimeCss || false,
      opt_ext || null);

  // Styles aren't always available synchronously. E.g. if there is a
  // pending style download, it will have to finish before the new
  // style is visible.
  // For this reason we poll until the style becomes available.
  // Sync case.
  if (styleLoaded(doc, style)) {
    cb(style);
    return style;
  }
  // Poll until styles are available.
  const interval = setInterval(() => {
    if (styleLoaded(doc, style)) {
      clearInterval(interval);
      cb(style);
    }
  }, 4);
  return style;
}

function styleLoaded(doc, style) {
  const sheets = doc.styleSheets;
  for (let i = 0; i < sheets.length; i++) {
    const sheet = sheets[i];
    if (sheet.ownerNode == style) {
      return true;
    }
  }
  return false;
};

insertStyleElement effectively does:

  const style = doc.createElement('style');
  style./*OK*/textContent = cssText;
  document.head.appendChild(style);

 

Comment 1 by nainar@chromium.org, May 14 2017

Labels: -Pri-3 Pri-2
Status: Available (was: Untriaged)

Comment 2 by nainar@chromium.org, May 15 2017

Labels: Hotlist-CodeHealth

Comment 3 by nainar@chromium.org, May 15 2017

Labels: Update-Quarterly
Cc: -r...@opera.com futhark@chromium.org
Labels: -Update-Quarterly

Comment 6 by cnardi@chromium.org, May 16 2018

I bet this is the same issue as why external/wpt/css/cssom/ttwf-cssom-doc-ext-load-count.html fails. Also fails on WebKit, but passes on Firefox/Edge.
The test is failing because it adds <link rel="stylesheet"> dynamically, and
immediately after that it's checking the length of document.styleSheets
which is 0 (against expected 1).
Once loading of the stylesheet finishes (or fails), the length becomes 1.

Scripts can know when the loading finished via 'load' or 'error' event,
and it should work interoperably.

For the implementation and behavior, as Firefox/Edge passing the test,
they may be just attach empty stylesheet to the element (and thus to document),
and fill the content later upon loading complete.

CSSOM spec says:
https://drafts.csswg.org/cssom/#ref-for-document-css-style-sheets%E2%91%A4
https://drafts.csswg.org/cssom/#requirements-on-specifications
> Specifications introducing new ways of associating style sheets through
> the DOM should define which nodes implement the LinkStyle interface.
> When doing so, they must also define when a CSS style sheet is created.

WHATWG HTML:
https://html.spec.whatwg.org/#link-type-stylesheet
https://html.spec.whatwg.org/#the-style-element
Both says
"Create a CSS style sheet with the following properties:...
  CSS rules
    Left uninitialized"

So from reading the doc, creating the object and filling CSS rules later upon
loding + parsing complete sounds more spec compliant.
Further research found
https://github.com/whatwg/html/issues/2997
https://github.com/whatwg/html/issues/696
and the specs are still unclear about this.

Sign in to add a comment