1374045 - Consider adding support for customizing scrolling behavior with Element.focus

Status: RESOLVED FIXED

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

Description

[Olli Pettay [:smaug][[e-mail]]]

There is no spec yet for this, so better to wait until there is, and then
review the spec carefully.

https://github.com/whatwg/html/issues/834

Comment 1

[Kartikaya Gupta (email:[e-mail])]

Chromium intent to implement: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/QeiK3lAXcmI

Comment 2

[Jihye Hong]

There is a spec proposal to disable automatic scroll into view on Element.focus().

Here is the summary of current situation: https://github.com/whatwg/html/pull/2787#issuecomment-338193107

The new dictionary type "FocusOptions" with the "preventScroll" dictionary member is introduced.

The IDL of Element.focus() will change to: 

dictionary FocusOptions {
  boolean preventScroll = false;
};
void focus(optional FocusOptions options);

If preventScroll is omitted or false, then the element will be scrolled into view with UA-defined manners.
Otherwise, it disables scrolling triggered by focus().

Comment 3

[Jihye Hong]

Related tests:
https://github.com/w3c/web-platform-tests/pull/7915
https://github.com/w3c/web-platform-tests/pull/7917

Comment 4

[alexander]

Very needed feature as preventScroll is extremely hard to implement properly on your own.

Comment 6

[Mirko Brodesser (:mbrodesser-Igalia)]

Filed a spec issue (https://github.com/whatwg/html/issues/4512) to avoid interoperability problems with Chrome and potentially clarify the spec for repeated re-focus events.

Comment 7

[Mirko Brodesser (:mbrodesser-Igalia)]

(Copying to here, because erroneously posted at the duplicate issue): Should be done for SVGElement too (https://html.spec.whatwg.org/#htmlorsvgelement). To keep the code simple we'll also add it to XULElement.

Comment 8

[Mirko Brodesser (:mbrodesser-Igalia)]

Attachment: Bug 1374045: add 'preventScroll' option to HTMLElement's, SVGElement's and XULElement's 'focus' method

• Remove expectation that 'preventScroll.html' fails.

• Use '[NoInterfaceObject] interface' workaround to simulate missing 'mixin' support.

Comment 9

[Mirko Brodesser (:mbrodesser-Igalia)]

A try run corresponding to the review: https://treeherder.mozilla.org/#/jobs?repo=try&revision=10b210d3331a4a5b00aa9c79247f37cc61722f73&selectedJob=239606015. There few failures are unrelated. Requesting check-in.

Comment 10

[Razvan Maries]

Tried to land this and received the following message: We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again. (255, 'applying /tmp/tmpzDM1eH\npatching file dom/html/HTMLLegendElement.cpp\nHunk #3 FAILED at 88\n1 out of 3 hunks FAILED -- saving rejects to file dom/html/HTMLLegendElement.cpp.rej\npatching file dom/html/HTMLLabelElement.cpp\nHunk #1 FAILED at 49\n1 out of 1 hunks FAILED -- saving rejects to file dom/html/HTMLLabelElement.cpp.rej\npatching file dom/base/Element.cpp\nHunk #1 FAILED at 337\n1 out of 1 hunks FAILED -- saving rejects to file dom/base/Element.cpp.rej\nabort: patch failed to apply', '')

Comment 11

[Mirko Brodesser (:mbrodesser-Igalia)]

Razvan: did you take the latest version of https://phabricator.services.mozilla.com/D26922? I had to resolve merge conflicts for the first version. I've just locally pulled and rebased without conflicts the latest version on:

commit 9dbbb9cb30e42518f410fb5b9968c7b178185748 (origin/branches/default/0b4aefa9f91cb941071c1fdb1c520188522bf710, origin/bookmarks/inbound, refs/cinnabar/refs/heads/branches/default/0b4aefa9f91cb941071c1fdb1c520188522bf710, refs/cinnabar/refs/heads/bookmarks/inbound)
Merge: 2782be40ae26 2ef7282d458b
Author: Ciure Andrei <[e-mail]>
Date:   Thu Apr 11 12:57:28 2019 +0300

    Merge mozilla-central to mozilla-inbound.  a=merge CLOSED TREE

Isn't the change cherry-picked on top of "origin/bookmarks/inbound"?

Comment 12

[Razvan Maries]

Phabricator lands patches on the top of autoland not inbound.

Comment 13

[Mirko Brodesser (:mbrodesser-Igalia)]

Razvan: I see. The merge-conflict stems from https://hg.mozilla.org/integration/autoland/rev/c5898e18dedf. I'll look into it.

Comment 14

[Mirko Brodesser (:mbrodesser-Igalia)]

A try run corresponding to the review: https://treeherder.mozilla.org/#/jobs?repo=try&revision=aa930ee0286e1af236f0be8dab8eb8dfab488a0c. The failures are unrelated to this change. Requesting check-in.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/894487fe3fa0
add 'preventScroll' option to HTMLElement's, SVGElement's and XULElement's 'focus' method r=smaug

Comment 16

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/894487fe3fa0

Comment 17

[Eric Shepherd [:sheppy]]

Documentation updates:

• While it already had most of this information, I have cleaned up and finished the changes to HTMLElement.focus()
• Submitted BCD PR 4359 to add and update the info for SVGElement/HTMLElement focus() method and its options