Bug 183889 - [iOS] Make the Find UI better estimate the obscured area when revealing highlighted text
Summary: [iOS] Make the Find UI better estimate the obscured area when revealing highl...
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Frames (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
Depends on: 183658
  Show dependency treegraph
Reported: 2018-03-21 23:28 PDT by Frédéric Wang (:fredw)
Modified: 2018-03-28 07:49 PDT (History)
1 user (show)

See Also:

Testcase (main frame) (10.37 KB, text/html)
2018-03-28 07:49 PDT, Frédéric Wang (:fredw)
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) 2018-03-21 23:28:49 PDT
In bug 183658, I landed a quick workaround to prevent the NBC news header from overlapping highlighted text result. Basically, we now just center the result in the viewport. However, there is probably a better way to estimate the obscured area via adjustScrollStepForFixedContent.
Comment 1 Frédéric Wang (:fredw) 2018-03-21 23:36:40 PDT
Sorry, I had forgotten to set a title.
Comment 2 Frédéric Wang (:fredw) 2018-03-27 04:24:08 PDT
I took a look into this. As I understand the logic of adjustScrollStepForFixedContent is to check if some fixed opaque content covers the full width at the top/bottom of the frame so that vertical page scroll can be adjusted appropriately on macOS.

I guess we can re-use this logic but we have couple differences here. First since fixed content is relative to parent subframe, adjusting the page scroll of "overflow: auto" node does not seem to work in general (we would have to scroll the parent subframe to make the overflow node visible). Second, I'm not exactly sure page scroll makes sense on iOS and anyway here it's not page scroll but "scroll to reveal match" which ends up at scrollRectToVisible.

Finally, in the AMP use case of bug 183658, the frame area is essentially the same as the "overflow: auto" area so we could apply the same logic but I wonder whether we should rather focus on making frame scrollable on iOS (bug 149264) in order to address these kinds of use case of "scrollable page with fixed-positioned header/footer".
Comment 3 Frédéric Wang (:fredw) 2018-03-28 07:49:05 PDT
Created attachment 336658 [details]
Testcase (main frame)

This is a testcase with the main frame. As I see:

* For Ctrl+F:
  * On macOS, the text is revealed without scrolling the result away from the "position: fixed" overlay (so similar to iOS before bug 183658).  
  * On iOS, the result is revealed at the center (so consistent with what is now done for overflow node after bug 183658)

* For PageUp/PageDown:
  * On macOS, the header/footer are taken into account in adjustScrollStepForFixedContent to calculate the page step value.
  * On iOS, it's not clear to me. The scroll is initiated in the UI process while FrameView::adjustScrollStepForFixedContent (or even ScrollArea::scroll) is not called in the Web process.