Bug 171883

Summary:

REGRESSION (r207372) Visibility property is not inherited when used in an animation

Product:

WebKit

Reporter:

Malte Ubl <malte>

Component:

CSS

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Critical

CC:

buildbot, commit-queue, dino, koivisto, manian, rbyers, rniwa, ryanhaddad, simon.fraser, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

Safari 10

Hardware:

All

OS:

All

Attachments:

Description	Flags
10.2 does not repro. Animated GIF showing the behavior.	none
patch	buildbot: commit-queue-
Archive of layout-test-results from ews103 for mac-elcapitan	none
patch	simon.fraser: review+
patch	none
patch	none

Description Malte Ubl 2017-05-09 14:04:22 PDT

It appears that the latest Safari stable release broke animation of CSS visibility.

Previously (like in other browser) visibility was correctly inherited by children. This is no longer the case.

Compare http://output.jsbin.com/yirehac/quiet between Safari versions (and Chrome and Firefox).

Repro case
<!doctype html>
<html>
<head>
  <style>
    
  body{
    animation:test 8s steps(1,end) 0s 1 normal both
  }
  @keyframes test {
    from{visibility:hidden}
    to{visibility:visible}
  }    
  body {
    background: #cfd8dc;
  }
  div {
    background: white; 
    width: 360px;
    height: 360px;
  }
  </style>  
</head>
<body>Hello, world.
  <div>
    Why am I shown?
  </div>
</body>
</html>

This bug leads to FOUC on many AMP pages (which at this point is a significant portion of internet traffic). It cannot be worked around in AMP as the relevant CSS is part of the distributed pages and not centrally managed. We can update the future recommendation, but the current 2 billion pages have the problem.

Comment 1 Radar WebKit Bug Importer 2017-05-09 14:26:17 PDT

<rdar://problem/32086550>

Comment 2 Simon Fraser (smfr) 2017-05-09 14:42:38 PDT

Is there a Safari version in which this bug did not occur? I can reproduce back to r210000.

Comment 3 Malte Ubl 2017-05-09 14:46:38 PDT

Created attachment 309545 [details]
10.2 does not repro. Animated GIF showing the behavior.

Comment 4 Simon Fraser (smfr) 2017-05-09 14:49:00 PDT

I was testing on Mac. It would be surprising if Mac and iOS behavior differed.

Comment 5 Malte Ubl 2017-05-09 14:51:45 PDT

Unfortunately I don't have a way to test on an older version of desktop Safari. It does repro in current mobile Safari and it does not repro in iOS 10.2 mobile Safari.

Comment 6 Simon Fraser (smfr) 2017-05-09 15:00:51 PDT

This regressed with https://trac.webkit.org/changeset/207372/webkit

Comment 7 Simon Fraser (smfr) 2017-05-09 15:02:36 PDT

I did ask about inherit with this change:
https://bugs.webkit.org/show_bug.cgi?id=163452#c3

Comment 8 Antti Koivisto 2017-05-09 15:57:22 PDT

I don't see 'inherit' in this test so I'm not sure what the relevance of that is.

Comment 9 Simon Fraser (smfr) 2017-05-09 16:23:51 PDT

This is possibly about the propagation of visibleContent bits in RenderLayers

Comment 10 Antti Koivisto 2017-05-09 16:30:06 PDT

Yeah, something like that seems likely. There shouldn't be anything special about visibility inheritance from style perspective.

Comment 11 Antti Koivisto 2017-05-10 03:23:45 PDT

Other inherited properties have the same problem. You can replace 'visibility' with 'color' here and get the same wrong behavior.

Comment 12 Antti Koivisto 2017-05-10 03:46:20 PDT

The problem here is that our animation code is tied to renderers. We don't have renderers during the initial style resolution so animations are not applied yet. When constructing renderers we set their style to the initial animated style but this step can't implement inheritance.

Normally this is invisible as the first animation frame will immediately inherit the style correctly. However in this case the animation is discrete and the first frame is the same as the initial state. With r207372 we optimize the descendant style change away.

There are simple fixes for this specific case. However at some point we should fix the underlying issues too.

Comment 13 Antti Koivisto 2017-05-10 05:22:28 PDT

Created attachment 309591 [details]
patch

Comment 14 Build Bot 2017-05-10 06:30:55 PDT

Comment on attachment 309591 [details]
patch

Attachment 309591 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/3711640

New failing tests:
animations/animation-initial-inheritance.html

Comment 15 Build Bot 2017-05-10 06:30:56 PDT

Created attachment 309599 [details]
Archive of layout-test-results from ews103 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

Comment 16 Antti Koivisto 2017-05-10 07:31:13 PDT

Created attachment 309603 [details]
patch

Comment 17 Simon Fraser (smfr) 2017-05-10 08:16:24 PDT

Comment on attachment 309603 [details]
patch

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

> Source/WebCore/style/StyleTreeResolver.cpp:264
> +            // FIXME: We should compute animated style correctly during initial style resolution when we don't have renderers yet.

Could you file a bug for this and reference it here please?

Comment 18 Antti Koivisto 2017-05-10 09:10:12 PDT

Created attachment 309612 [details]
patch

Comment 19 WebKit Commit Bot 2017-05-10 09:50:59 PDT

Comment on attachment 309612 [details]
patch

Clearing flags on attachment: 309612

Committed r216591: <http://trac.webkit.org/changeset/216591>

Comment 20 WebKit Commit Bot 2017-05-10 09:51:01 PDT

All reviewed patches have been landed.  Closing bug.

Comment 21 Ryan Haddad 2017-05-10 14:14:22 PDT

Reverted r216591 for reason:

This change broke an internal build.

Committed r216619: <http://trac.webkit.org/changeset/216619>

Comment 22 Antti Koivisto 2017-05-10 14:58:33 PDT

Created attachment 309643 [details]
patch

updated SameSizeAsRenderElement struct to accommodate an additional bit

Comment 23 WebKit Commit Bot 2017-05-10 15:23:15 PDT

Comment on attachment 309643 [details]
patch

Clearing flags on attachment: 309643

Committed r216631: <http://trac.webkit.org/changeset/216631>

Comment 24 WebKit Commit Bot 2017-05-10 15:23:16 PDT

All reviewed patches have been landed.  Closing bug.

Comment 25 Malte Ubl 2017-05-11 02:47:53 PDT

Thanks for the super quick fix!