Element.focus() should scroll to the position where scroll-margin and scroll-padding values are factored into
Categories
(Core :: Layout: Scrolling and Overflow, enhancement, P3)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox68 | --- | fixed |
People
(Reporter: hiro, Assigned: hiro)
References
Details
Attachments
(1 file)
The spec clearly says Elemnt.focus() should do scroll the element into view, but nsFocusManager::ScrollIntoView which is called from Element.focus() is also used for Find in page functionality. The Find in page scrolls to the position where the target element is positioned at the bottom of the page, it's intentional (see bug 720126). So, in the Find in page case, we want to ignore scroll-margin and scroll-padding values, that means we need to add a new flag in nsIFocusManager.idl (both of Element.focus() and Find in page call nsIFocusManager.setFocus), so it's better to modify it in a different bug from 1534070).
|
Assignee | |
Comment 1•5 years ago
|
||
After some more thought about this, I think now Find in page should also factor the scroll-padding value into the position when we scroll to the found text in pages, since the scroll-snap spec says "This allows the author to exclude regions of the scrollport that are obscured by other content (such as fixed-positioned toolbars or sidebars)" so that if site authors specify scroll-padding properly, we can avoid fixed (or sticky) positioned elements cover the found text such as bug 622801.
mikedeboer, what do you think? (I am not sure you are the right person to ask, but I can see you are the triage owner of Find Toolbar)
|
||
Comment 2•5 years ago
|
||
hiro-san, I think you're right. Please feel free to unify this behavior; it feel odd to have this behave differently just for find in page.
|
|
Assignee | |
Comment 3•5 years ago
|
||
Thank you, Mike de Boer! I will do it.
|
|
Assignee | |
Comment 4•5 years ago
|
||
We also take account those values in the case of Find in page.
The corresponding web platform tests will be coming from this PR.
https://github.com/web-platform-tests/wpt/pull/8575
Though some of them will not be passed, the failure reason is not related
to this change, I will take a look when the PR gets merged into mozilla-central.
Pushed by hikezoe@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/c5898e18dedf Take account the scroll-snap-margin and scroll-snap-padding into the position where we scroll to on Element.focus() call. r=masayuki,botond
|
||
Comment 6•5 years ago
|
||
| bugherder | ||
Description
•