[webkit-reviews] review canceled: [Bug 208352] Page::editableElementsInRect() should find nested editable elements and return found elements in paint order : [Attachment 392075] Patch

[bugzilla-daemon at webkit.org]

Daniel Bates <dbates at webkit.org> has canceled Daniel Bates
<dbates at webkit.org>'s request for review:
Bug 208352: Page::editableElementsInRect() should find nested editable elements
and return found elements in paint order
https://bugs.webkit.org/show_bug.cgi?id=208352

Attachment 392075: Patch

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




--- Comment #4 from Daniel Bates <dbates at webkit.org> ---
Comment on attachment 392075
  --> https://bugs.webkit.org/attachment.cgi?id=392075
Patch

Patch does not address the iOS test failure and more likely the updated results
will cause more iOS test failures. I am very suspicious of "[webView
scrollView].contentOffset = ..." in the test code. I would have expected
window.scrollTo() to also work on iOS. In fact, as I'm typing this now and
thinking about how iOS is currently failing I think setting .contentOffset will
never produce good results because the engine is never told about the scroll
position change so it cannot update its visible content rect and hit testing
will fail.

The kind of failure seen on the bots in the earlier patch: the test is hit
testing at the top of the document (scrolled offscreen). As to why the test
passes before my change. I can answer that: the original code was ASYMMETRIC!
Walking the DOM will find things **outside the visible viewport** whereas hit
testing will NOT, by default.