Bug 149157 – Allow the page to render before <link> stylesheet tags in body

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Jake Archibald]

Test page: https://wiki-offline.jakearchibald.com/wiki/Hulk_Hogan?use-url-flags&prevent-sw=1&no-css-async-hack=1

Results:
Chrome http://www.webpagetest.org/video/compare.php?tests=150424_57_12PJ-r:1-c:0
IE http://www.webpagetest.org/video/compare.php?tests=150424_NH_12XE-r:1-c:0

In the test above, the page has inline CSS, 'defer'd javascript, then the HTML for the top toolbar. After the toolbar, there's a <link> for some CSS (load.php in the waterfall), followed by the article content.

In Chrome/Safari, the preparser discovers the CSS and blocks further rendering, including content before the <link>. IE will allow content before the <link> to render, and block afterwards. Firefox is similar to IE

The spec is unclear here, but developers seem to want this behaviour https://www.w3.org/Bugs/Public/show_bug.cgi?id=27303#c37, and there are hacks to work around the current block-everything behaviour https://github.com/filamentgroup/loadCSS

Chrome is looking to align with Firefox & IE on this https://code.google.com/p/chromium/issues/detail?id=481122#c17

Further explanation of the problem & benefits of this solution https://www.w3.org/Bugs/Public/show_bug.cgi?id=27303#c36

[Jake Archibald]

I realise the videos in the previous comment don't show the effect in Safari specifically, unfortunately WPT doesn't support it, but testing with the Network Link Conditioner shows it to have the same issue as Chrome.

[Simon Fraser (smfr)]

Chrome is planning to ship this:
https://jakearchibald.com/2016/link-in-body/

[Radar WebKit Bug Importer]

<rdar://problem/24658830>

[Antti Koivisto]

> In Chrome/Safari, the preparser discovers the CSS and blocks further
> rendering, including content before the <link>. 

This is not true in WebKit. Stylesheet loads triggered by preload scanner don't block rendering. Rendering is only blocked when the real parser reaches a loading <link> element (or <style> with @import).

[Antti Koivisto]

Created attachment 303826 [details]
patch

[Antti Koivisto]

*** Bug 169345 has been marked as a duplicate of this bug. ***

[Simon Fraser (smfr)]

Comment on attachment 303826 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=303826&action=review

> Source/WebCore/rendering/RenderBlock.cpp:1575
> +    // Style is non-final if the element has a pending stylesheet before it. We end up with renderers with such styles if a script
> +    // forces renderer construction by querying something layout dependent.

Is this comment accurate with <link rel="stylesheet"> in the body? Won't we force renderer construction anyway?

> Source/WebCore/style/StyleTreeResolver.cpp:171
> +        m_document.setHasNodesWithNonFinalStyle();

Wehre is this cleared? I see no calls to m_document.setHasNodesWithNonFinalStyle(false)

> Source/WebCore/style/StyleTreeResolver.cpp:184
> +        m_document.setHasNodesWithNonFinalStyle();

Ditto.

[Antti Koivisto]

> Is this comment accurate with <link rel="stylesheet"> in the body? Won't we
> force renderer construction anyway?

In normal case we only compute style (and so construct renderers) up to the loading in-body stylesheet. Call to updateLayoutIgnorePendingStylesheet() forces renderer construction for all elements. In that case we generate renderers with non-final style.

> Wehre is this cleared? I see no calls to
> m_document.setHasNodesWithNonFinalStyle(false)

Document::resolveStyle() has

m_hasNodesWithNonFinalStyle = false;

[Antti Koivisto]

> Document::resolveStyle() has
> 
> m_hasNodesWithNonFinalStyle = false;

...when we do full style rebuild.

[Simon Fraser (smfr)]

(In reply to comment #9)
> > Document::resolveStyle() has
> > 
> > m_hasNodesWithNonFinalStyle = false;
> 
> ...when we do full style rebuild.

It would make searching easier if this called the setHasNodesWithNonFinalStyle() function

[Antti Koivisto]

> It would make searching easier if this called the
> setHasNodesWithNonFinalStyle() function

I don't want to make setHasNodesWithNonFinalStyle take an argument since no one except Document itself should reset the flag.

[WebKit Commit Bot]

Comment on attachment 303826 [details]
patch

Clearing flags on attachment: 303826

Committed r213633: <http://trac.webkit.org/changeset/213633>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Ryan Haddad]

*** Bug 169345 has been marked as a duplicate of this bug. ***

[Ryan Haddad]

Reverted r213633 for reason:

This change caused LayoutTest imported/w3c/web-platform-tests/html/semantics/embedded-content/the-img-element/sizes/parse-a-sizes-attribute.html to become a flaky failure.

Committed r213701: <http://trac.webkit.org/changeset/213701>

[Ryan Haddad]

*** Bug 169452 has been marked as a duplicate of this bug. ***

[Antti Koivisto]

Fix for bug 169465 should make imported/w3c/web-platform-tests/html/semantics/embedded-content/the-img-element/sizes/parse-a-sizes-attribute.html reliable.

[WebKit Commit Bot]

Comment on attachment 303826 [details]
patch

Rejecting attachment 303826 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-02', 'validate-changelog', '--check-oops', '--non-interactive', 303826, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

ChangeLog entry in LayoutTests/ChangeLog contains OOPS!.

Full output: http://webkit-queues.webkit.org/results/3283697

[Antti Koivisto]

Created attachment 304042 [details]
patch

[WebKit Commit Bot]

Comment on attachment 304042 [details]
patch

Clearing flags on attachment: 304042

Committed r213712: <http://trac.webkit.org/changeset/213712>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Antti Koivisto]

Reverted in http://trac.webkit.org/changeset/214333 due to iPad PLT regression.

[Antti Koivisto]

Created attachment 305510 [details]
patch

[WebKit Commit Bot]

Comment on attachment 305510 [details]
patch

Clearing flags on attachment: 305510

Committed r214435: <http://trac.webkit.org/changeset/214435>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.