Bug 179315 - Content not painted when scrolling an overflow node inside an iframe
Summary: Content not painted when scrolling an overflow node inside an iframe
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Frames (show other bugs)
Version: WebKit Nightly Build
Hardware: iPhone / iPad iOS 11
: P2 Normal
Assignee: Simon Fraser (smfr)
URL:
Keywords: InRadar, Regression
Depends on:
Blocks:
 
Reported: 2017-11-06 03:24 PST by Frédéric Wang (:fredw)
Modified: 2017-11-10 09:51 PST (History)
8 users (show)

See Also:


Attachments
Testcase (845 bytes, text/html)
2017-11-06 03:24 PST, Frédéric Wang (:fredw)
no flags Details
Patch (5.24 KB, patch)
2017-11-08 17:38 PST, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews123 for ios-simulator-wk2 (2.22 MB, application/zip)
2017-11-08 20:07 PST, Build Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Frédéric Wang (:fredw) 2017-11-06 03:24:13 PST
Created attachment 326110 [details]
Testcase

The attached testcase contains an overflow node inside an iframe. The overflow itself node contains some green rectangle. On iOS 11, the green rectangle is not visible/painted when you scroll the overflow node down. Note that changing a bit the CSS or HTML content may make the green rectangle visible. 

This issue was initially reported in https://github.com/ampproject/amphtml/issues/11829 ; and the testcase was reduced from https://output.jsbin.com/veficav/quiet
Comment 1 Frédéric Wang (:fredw) 2017-11-06 03:26:40 PST
Per the original github issue, this is a regression as it affects iOS 11, not iOS 10 (I did not verify this assertion though).
Comment 2 Radar WebKit Bug Importer 2017-11-06 07:18:27 PST
<rdar://problem/35364166>
Comment 3 Dima Voytenko 2017-11-06 10:55:05 PST
I'm seeing this bug in different combinations quite a bit. It looks like a very large regression.
Comment 4 Simon Fraser (smfr) 2017-11-06 10:57:48 PST
Are you seeing issues scrolling inside overflow:scroll in UIWebView? If so, that's bug 179315
Comment 5 Frédéric Wang (:fredw) 2017-11-06 11:12:59 PST
(In reply to Simon Fraser (smfr) from comment #4)
> Are you seeing issues scrolling inside overflow:scroll in UIWebView? If so,
> that's bug 179315

Which bug did you mean to mention?
Comment 6 Simon Fraser (smfr) 2017-11-06 11:18:47 PST
Sorry, bug 179277.
Comment 7 Frédéric Wang (:fredw) 2017-11-06 11:35:09 PST
(In reply to Simon Fraser (smfr) from comment #6)
> Sorry, bug 179277.

At least for attachment 326110 [details]  and https://output.jsbin.com/veficav/quiet, your patch does not fix the problem.
Comment 8 Justin Ridgewell 2017-11-06 11:36:39 PST
I was able to pear down the initial AMP bug report to https://output.jsbin.com/pehorog/28

This produces **without** an iframe, just using any ole document with overflow-scrolling: touch, an div with overflows, and position: relatives.
Comment 9 Simon Fraser (smfr) 2017-11-06 11:37:18 PST
Are you folks testing in Safari or UIWebView?
Comment 10 Justin Ridgewell 2017-11-06 11:42:17 PST
Directly in Safari for me.
Comment 11 Justin Ridgewell 2017-11-06 11:47:48 PST
To be explicit:

"Mozilla/5.0 (iPhone; CPU iPhone OS 11_0_3 like Mac OS X) AppleWebKit/604.1.38 (KHTML, like Gecko) Version/11.0 Mobile/15A432 Safari/604.1" on an iPhone SE.

"Mozilla/5.0 (iPhone; CPU iPhone OS 11_0_3 like Mac OS X) AppleWebKit/604.1.38 (KHTML, like Gecko) Version/11.0 Mobile/15A432 Safari/604.1" on an iPhone 6.
Comment 12 Simon Fraser (smfr) 2017-11-06 11:58:37 PST
For me the last line pops in after a delay, but it doesn't look great. Feels similar to   bug 179277.
Comment 13 Justin Ridgewell 2017-11-06 12:34:38 PST
Ok, the "pops in after a delay" is actually because of the little "Edit in JSBin" button that disappears, causing a recalc. If you haven't scrolled to view the text before it disappears, the text will remain hidden.

Or, You can visit https://jsbin.com/pehorog/28/quiet, which doesn't have the disappearing JSBin button.

I've also tested this in iOS 9.3.4 (iPhone 6) and iOS 10.2.1 (iPhone 5C), and they produce the same hidden text.
Comment 14 Dima Voytenko 2017-11-07 08:55:35 PST
Agree with Justin, https://jsbin.com/pehorog/28/quiet repros ~100% of the time and doesn't fix itself. Reflow will eventually fix it if that happens. This repro seems very common. Maybe it's worth raising priority?
Comment 15 Frédéric Wang (:fredw) 2017-11-07 09:07:20 PST
(In reply to Simon Fraser (smfr) from comment #9)
> Are you folks testing in Safari or UIWebView?

Yes, I was testing on Safari too. As I said your fix for the UIWebView did not help, so it's a similar but different regression. I tried bisecting it but it is difficult to build WebKit for iOS 11 on old versions with the public SDK :-( I'll try refining the regression window, but for now:

* Safari 10.2.1 (14D27) ; iOS 10 does not have the bug (according to the original reporter).
* WebKit r220111 (Aug 1) ; iOS 11 has the bug ( https://trac.webkit.org/changeset/220111 ) 

Also, I'm not sure whether the regression is in WebKit or iOS or both.
Comment 16 Simon Fraser (smfr) 2017-11-07 09:25:49 PST
It's probably a WebKit regression, related to dropping backing store for invisible elements. I'll take a look.
Comment 17 Simon Fraser (smfr) 2017-11-08 13:37:12 PST
The missing content is in its own compositing layer (because of the position:relative and other composited content z-ordered behind it).
Comment 18 Justin Ridgewell 2017-11-08 14:36:17 PST
Is there anything AMP can do to avoid this issue? Or, anything we can do to detect/fix it? I was able to hack together a fix by applying transform: translateZ(0) to _anything_ with relative/absolute positioning. But that's super expensive to detect, isn't going to work for every page (position: fixed springs to mind), and (I'd imagine) expensive for Safari to render.

We're very worried this affecting a larger percentage of AMP docs. We frequently get internal communications from publishers with "broken" pages due to GPU issues in iOS. One affected every (!!!) Pinterest page in search.

AMP allows publisher provided CSS on the page, but otherwise controls the entire document from "default" CSS (where we can override anything by using !important) and we're the only JS running on the page. Basically any fix you can think of, we can implement. We're just looking for the right approach.
Comment 19 Simon Fraser (smfr) 2017-11-08 15:57:06 PST
I'll tell you when I've figured out what causes it.
Comment 20 Simon Fraser (smfr) 2017-11-08 16:22:59 PST
In the stacking order tree, <h1 class="test"> is a sibling of <div class="card">, and should be painted later. <h1 class="test"> gets composited, so normally <div class="card"> would also get composited because of overlap. However, I think we detect that <div class="card"> is outside the viewport, so don't composite it. The problem is that it gets revealed by overflow scrolling and we don't run the overlap testing logic again.

A simple fix is to make <div class="card"> a stacking context by adding z-index:0
Comment 21 Simon Fraser (smfr) 2017-11-08 16:39:23 PST
Ah, we don't check for overlap changes on composited scroll, and should
Comment 22 Simon Fraser (smfr) 2017-11-08 17:38:41 PST
Created attachment 326413 [details]
Patch
Comment 23 Build Bot 2017-11-08 20:07:41 PST
Comment on attachment 326413 [details]
Patch

Attachment 326413 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/5157137

New failing tests:
http/tests/workers/service/service-worker-clear.html
Comment 24 Build Bot 2017-11-08 20:07:42 PST
Created attachment 326424 [details]
Archive of layout-test-results from ews123 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.6
Comment 25 WebKit Commit Bot 2017-11-08 21:35:38 PST
Comment on attachment 326413 [details]
Patch

Clearing flags on attachment: 326413

Committed r224618: <https://trac.webkit.org/changeset/224618>
Comment 26 WebKit Commit Bot 2017-11-08 21:35:40 PST
All reviewed patches have been landed.  Closing bug.
Comment 27 Frédéric Wang (:fredw) 2017-11-10 09:07:25 PST
@Simon: Thanks for your patch. I just tried the two test cases again with a fresh build and I can confirm the bugs are fixed.
Comment 28 Dima Voytenko 2017-11-10 09:51:34 PST
Thanks so much for the quick turn around!