Open Bug 403510 Opened 13 years ago Updated 2 years ago

Implement scrollIntoViewIfNeeded

Categories

(Core :: DOM: Core & HTML, enhancement, P5)

enhancement

Tracking

()

People

(Reporter: erik, Unassigned)

References

Details

(Keywords: dev-doc-needed, parity-chrome, parity-safari)

Attachments

(1 file, 1 obsolete file)

Newer WebKit builds has a very useful method on HTMLElement called:

scrollIntoViewIfNeeded(opt_center : boolean)

It will scroll an element into view if it is outside the viewport. The param, if true or left out will center the element and if false it will scroll as little as possible to make the element visible in the viewport.

This is very similar to scrollIntoView(opt_top) except that it will not scroll any elements if the element is already fully visible in the viewport.
Blocks: 703346
Is there a specification for this method?
If not, can we consider improving it?

> scrollIntoViewIfNeeded(alignWithTop, center);

if center is true, and if the node is shorter than the page, the node is centered in the middle of the page. And that would keep it compatible with Webkit.
Disregard comment 1. It's already addressed in the Description.
Blocks: 724071
Attached patch Proposed fix. (obsolete) — Splinter Review
Attachment #594312 - Flags: feedback?(paul)
Comment on attachment 594312 [details] [diff] [review]
Proposed fix.

Great start :)

>+  void scrollIntoViewIfNeeded([optional] in boolean top, [optional] in boolean centerIfNeeded);

The signature should be:
[optional_argc] void scrollIntoView([optional] in boolean top);

Look at description.
Attachment #594312 - Flags: feedback+
Comment on attachment 594312 [details] [diff] [review]
Proposed fix.

Great start :)

>+  void scrollIntoViewIfNeeded([optional] in boolean top, [optional] in boolean centerIfNeeded);

The signature should be:
[optional_argc] void scrollIntoView([optional] in boolean center);

Look at description.
Attachment #594312 - Flags: feedback+
Ignore comment #4, comment #5 is the one.
New fix taking into account Cedric's comments.
Attachment #594312 - Attachment is obsolete: true
Attachment #594427 - Flags: feedback?(paul)
Attachment #594312 - Flags: feedback?(paul)
Blocks: 724515
Comment on attachment 594427 [details] [diff] [review]
Fix more inline with Webkit's implementation.

Review of attachment 594427 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/html/content/src/nsGenericHTMLElement.cpp
@@ +1009,5 @@
> +  nsCOMPtr<nsIPresShell> presShell = document->GetShell();
> +  if (!presShell) {
> +    return NS_OK;
> +  }
> +

We need to return early if the element is already fully visible (that's the main rationale for the function: to reduce unnecessarily scrolling).
This scrolls the page to make it visible on the center of the page, but it doesn't address the "IfNeeded" part.
We should not take a patch for this bug before there is a specification. Please send email to www-style to propose it.
Agreed - please don't implement our random features that haven't had any external discussion yet.

(This feature sucks as implemented, for example - it continues the horrible practice of boolean flags.  We could handle this much better by extending scrollIntoView with an options object.)
Having a way to scroll the page to center an element is needed for a couple of our Devtools.

This method happens to do what we are looking for (in webkit).
This doesn't have to be exposed to the content.

I opened a more generic bug where we can discuss the best approach: bug 724585 - We need a way to scroll a page to center an element if the element is not visible
Attachment #594427 - Flags: feedback?(paul)
If it can help for some people I tried a little JS implementation of how WebKit does it : https://gist.github.com/2581101
I have suggested an implementation as a CSSOM-view bug report: <https://www.w3.org/Bugs/Public/show_bug.cgi?id=17152>.

I'd love to have feedback!
(In reply to Thaddee Tyl [:espadrine] from comment #14)
> I'd love to have feedback!

As a meta feedback, I would suggest you post that to www-style, where discussions happen. You don't need to subscribe the list in order to post to it, but some people might forget to put you in the Cc list. So, *shrug*.
Kang-Hao: thanks for the suggestion! I ended up subscribing to the list. I've started a thread, too!
No longer blocks: 703346
Whiteboard: [parity-chrome][parity-safari]
Mass bug change to replace various 'parity' whiteboard flags with the new canonical keywords. (See bug 1443764 comment 13.)
Whiteboard: [parity-chrome][parity-safari]
Priority: -- → P5
You need to log in before you can comment on or make changes to this bug.