[webkit-reviews] review denied: [Bug 194900] Add more tests for programmatic frame scrolling on iOS : [Attachment 362605] Patch

Fri Feb 22 10:24:17 PST 2019

Simon Fraser (smfr) <simon.fraser at apple.com> has denied Frédéric Wang (:fredw)
<fred.wang at free.fr>'s request for review:
Bug 194900: Add more tests for programmatic frame scrolling on iOS
https://bugs.webkit.org/show_bug.cgi?id=194900

Attachment 362605: Patch

https://bugs.webkit.org/attachment.cgi?id=362605&action=review

--- Comment #3 from Simon Fraser (smfr) <simon.fraser at apple.com> ---
Comment on attachment 362605
  --> https://bugs.webkit.org/attachment.cgi?id=362605
Patch

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

> LayoutTests/fast/scrolling/ios/hit-testing-iframe.html:110
> +	 <iframe id="clickElementInsideFrameAfterProgrammaticScroll"
style="left: 100px; top: 100px;" scrolling="yes"
onclick="this.style.background='red'" srcdoc="
> +	     <body style='margin: 0; width: 200px; height: 200px; background:
green;'>
> +		<div style='position: absolute; width: 75px; height: 75px;
background: red;'></div>
> +		 <div style='position: absolute; left: 110px; top: 110px;
width: 50px; height: 50px; background: red;'
> +		      onclick='this.style.background="green"'></div>
> +	     </body>" onload="newFrameLoaded()">
> +	 </iframe>

I don't like tests that do multiple different things. It's really hard to debug
them, and when the test breaks, you have to spend time investigating to see
which part broke. I think this should be broken into 6(?) different tests.