Bug 196394 - Add more tests for ios iframe scrolling with fixed/sticky layers
Summary: Add more tests for ios iframe scrolling with fixed/sticky layers
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: UI Events (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Frédéric Wang (:fredw)
URL:
Keywords:
Depends on: 192303 194886 197280
Blocks:
  Show dependency treegraph
 
Reported: 2019-03-29 03:12 PDT by Frédéric Wang (:fredw)
Modified: 2019-04-25 07:27 PDT (History)
2 users (show)

See Also:


Attachments
Patch (15.22 KB, patch)
2019-03-29 03:32 PDT, Frédéric Wang (:fredw)
no flags Details | Formatted Diff | Diff
Additional patch to use immediateScrollElementAtContentPointToOffset (2.84 KB, patch)
2019-03-29 09:00 PDT, Frédéric Wang (:fredw)
no flags Details | Formatted Diff | Diff
Patch (7.73 KB, patch)
2019-04-25 07:20 PDT, Frédéric Wang (:fredw)
no flags Details | Formatted Diff | Diff
Patch (alternative version using immediateScrollElementAtContentPointToOffset) (7.88 KB, patch)
2019-04-25 07:27 PDT, Frédéric Wang (:fredw)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Frédéric Wang (:fredw) 2019-03-29 03:12:18 PDT
I'll import the remaining tests I wrote for ios frame scrolling. Not sure why commits fixed the issues I had, but it seems they all pass now https://bugs.webkit.org/show_bug.cgi?id=194433#c14
Comment 1 Frédéric Wang (:fredw) 2019-03-29 03:32:22 PDT
Created attachment 366262 [details]
Patch
Comment 2 Antti Koivisto 2019-03-29 03:41:24 PDT
Comment on attachment 366262 [details]
Patch

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

> LayoutTests/fast/scrolling/ios/scroll-iframe-005.html:29
> +          // This checks scrolling to the location of the green square.
> +          var c = centerOf("positionFixed");
> +          await touchAndDragFromPointToPoint(c.x, c.y, c.x - 150, c.y - 150);
> +          await liftUpAtPoint(c.x - 150, c.y - 150);
> +
> +          // Wait for scrolling to stabilize and for scrollbars to disappear.
> +          setTimeout(() => {testRunner.notifyDone(); }, 1000);

Can you make these use UIHelper.immediateScrollElementAtContentPointToOffset and remove the timeouts? See

LayoutTests/fast/scrolling/ios/overflow-scroll-overlap.html (and LayoutTests/fast/scrolling/resources/overflow-scroll-overlap.js)

for an example.
Comment 3 Frédéric Wang (:fredw) 2019-03-29 09:00:08 PDT
Created attachment 366274 [details]
Additional patch to use immediateScrollElementAtContentPointToOffset
Comment 4 Frédéric Wang (:fredw) 2019-03-29 09:01:25 PDT
Comment on attachment 366262 [details]
Patch

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

>> LayoutTests/fast/scrolling/ios/scroll-iframe-005.html:29
>> +          setTimeout(() => {testRunner.notifyDone(); }, 1000);
> 
> Can you make these use UIHelper.immediateScrollElementAtContentPointToOffset and remove the timeouts? See
> 
> LayoutTests/fast/scrolling/ios/overflow-scroll-overlap.html (and LayoutTests/fast/scrolling/resources/overflow-scroll-overlap.js)
> 
> for an example.

Mmh, I tried it (attachment 366274 [details]) but that does not seem to work. Not sure which mistake I made...
Comment 5 Simon Fraser (smfr) 2019-03-29 14:07:11 PDT
Comment on attachment 366262 [details]
Patch

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

>>> LayoutTests/fast/scrolling/ios/scroll-iframe-005.html:29
>>> +          setTimeout(() => {testRunner.notifyDone(); }, 1000);
>> 
>> Can you make these use UIHelper.immediateScrollElementAtContentPointToOffset and remove the timeouts? See
>> 
>> LayoutTests/fast/scrolling/ios/overflow-scroll-overlap.html (and LayoutTests/fast/scrolling/resources/overflow-scroll-overlap.js)
>> 
>> for an example.
> 
> Mmh, I tried it (attachment 366274 [details]) but that does not seem to work. Not sure which mistake I made...

I think it's worth trying. You're adding 2s of tests, and that's not a sustainable rate.
Comment 6 Frédéric Wang (:fredw) 2019-04-01 12:24:09 PDT
(In reply to Simon Fraser (smfr) from comment #5)
> >> LayoutTests/fast/scrolling/ios/overflow-scroll-overlap.html (and LayoutTests/fast/scrolling/resources/overflow-scroll-overlap.js)
> >> 
> >> for an example.
> > 
> > Mmh, I tried it (attachment 366274 [details]) but that does not seem to work. Not sure which mistake I made...
> 
> I think it's worth trying. You're adding 2s of tests, and that's not a
> sustainable rate.

Yes, I agree with that. The thing is that I was not sure whether I made something wrong in my attempt attachment 366274 [details] as the change makes the test then fails when I tried the other day. Not sure why they pass with the other API.
Comment 7 Frédéric Wang (:fredw) 2019-04-25 06:40:45 PDT
(In reply to Simon Fraser (smfr) from comment #5)
> Comment on attachment 366262 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=366262&action=review
> 
> >>> LayoutTests/fast/scrolling/ios/scroll-iframe-005.html:29
> >>> +          setTimeout(() => {testRunner.notifyDone(); }, 1000);
> >> 
> >> Can you make these use UIHelper.immediateScrollElementAtContentPointToOffset and remove the timeouts? See
> >> 
> >> LayoutTests/fast/scrolling/ios/overflow-scroll-overlap.html (and LayoutTests/fast/scrolling/resources/overflow-scroll-overlap.js)
> >> 
> >> for an example.
> > 
> > Mmh, I tried it (attachment 366274 [details]) but that does not seem to work. Not sure which mistake I made...
> 
> I think it's worth trying. You're adding 2s of tests, and that's not a
> sustainable rate.

I'm still not able to make them pass with the internal APIs so I'll move this to a separate bug and handle the other tests here.
Comment 8 Frédéric Wang (:fredw) 2019-04-25 07:20:21 PDT
Created attachment 368235 [details]
Patch

Rebasing on top of bug 197280.
Comment 9 Frédéric Wang (:fredw) 2019-04-25 07:27:08 PDT
Created attachment 368236 [details]
Patch (alternative version using immediateScrollElementAtContentPointToOffset)

With that version, the tests fail for me.