Consider adding support for customizing scrolling behavior with Element.focus

RESOLVED FIXED in Firefox 68

Status

()

enhancement
P3
normal
RESOLVED FIXED
2 years ago
3 days ago

People

(Reporter: smaug, Assigned: mbrodesser)

Tracking

(Depends on 1 bug, {dev-doc-complete})

unspecified
mozilla68
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox68 fixed)

Details

Attachments

(1 attachment)

Reporter

Description

2 years ago
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
Priority: -- → P3

Comment 2

2 years ago
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 4

4 months ago

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

Component: DOM → DOM: Core & HTML
Duplicate of this bug: 1540588
Assignee

Updated

3 months ago
Assignee: nobody → mbrodesser
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Assignee

Comment 6

3 months ago

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.

Assignee

Comment 7

3 months ago

(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.

Assignee

Comment 8

3 months ago
  • Remove expectation that 'preventScroll.html' fails.

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

Assignee

Updated

2 months ago
Keywords: dev-doc-needed
Assignee

Comment 9

2 months ago

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.

Assignee

Updated

2 months ago
Keywords: checkin-needed

Comment 10

2 months ago

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', '')

Keywords: checkin-needed
Assignee

Comment 11

2 months ago

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 <aciure@mozilla.com>
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"?

Flags: needinfo?(rmaries)

Comment 12

2 months ago

Phabricator lands patches on the top of autoland not inbound.

Flags: needinfo?(rmaries)
Assignee

Comment 13

2 months ago

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

Assignee

Comment 14

2 months ago

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.

Assignee

Updated

2 months ago
Keywords: checkin-needed

Comment 15

2 months ago

Pushed by cbrindusan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/894487fe3fa0
add 'preventScroll' option to HTMLElement's, SVGElement's and XULElement's 'focus' method r=smaug

Keywords: checkin-needed

Comment 16

2 months ago
bugherder
Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
Depends on: 1544660

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
You need to log in before you can comment on or make changes to this bug.