New issue
Advanced search Search tips

Issue 734166 link

Starred by 12 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Feature



Sign in to add a comment

Enable ability to prevent scrolling in Element.focus()

Project Member Reported by dtapu...@chromium.org, Jun 16 2017

Issue description

There are various proposals regarding the ability to disable automatic scroll into view on Element.focus().

See:
https://github.com/whatwg/html/issues/834#issuecomment-305089508
 
Labels: -Type-Bug Type-Feature
Owner: eirage@chromium.org
Status: Assigned (was: Untriaged)
Ella once the spec has finalized you can go ahead and implement this. Navid will help getting the spec finalized. I believe LG is doing the spec PR.

Comment 3 Deleted

Comment 4 Deleted

Comment 5 by jihyer...@gmail.com, Oct 23 2017

The spec proposal related about this is about to be merged.

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

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

Comment 6 by hyojin22...@lge.com, Oct 31 2017

The feature has been finally merged in WHATWG spec.
https://html.spec.whatwg.org/multipage/interaction.html#focus-management-apis

Navid put a comment about a part of test cases, so it would be handled properly.
https://github.com/w3c/web-platform-tests/pull/7917

I wonder Ella could implement this soon. As Rick mentioned, it depends on the priority the team give it. If it needs more time to get started, LG can consider to contribute part of the implementation and its testing work.
Ella has already implemented the feature and also modified the test in her CL and I also reviewed the CL. She is just waiting for other owners approvals.

https://chromium-review.googlesource.com/c/chromium/src/+/731225


Quick question, would this let me do the following:

element.scrollIntoView({behavior: "smooth"});
element.focus({preventScroll: true });

Comment 9 by eirage@chromium.org, Oct 31 2017

Yes, you can use focus({preventScroll: true }) and scrollIntoView to achieve focus with scrolling behavior. (tested with my patch)
@robdodson, see "scrollOptions" spec proposal (I expect you'll not need for "scrollIntoView") https://github.com/whatwg/html/pull/2787

Element.focus({
    scrollOptions: {
        behavior: "auto" | "none" | "smooth",
        block: "start" | "end"
    }
);
I read through those threads but it doesn't appear that variant of scrollOptions is what will be shipping.

Per this comment: https://github.com/w3c/csswg-drafts/pull/1805#issuecomment-337801758
Great! If it gets the final approval, LGE will test our use cases after applying the changes to our branch. I expect it would work well.

The initial proposal described the "scrollOptions" with scroll behavior and offset, but it couldn't be merged due to several issues so that "preventScroll" property could only be merged as a first result.
Project Member

Comment 13 by bugdroid1@chromium.org, Nov 9 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d4b0ae2e7c827f152f94c0886465954b18590af0

commit d4b0ae2e7c827f152f94c0886465954b18590af0
Author: Ella Ge <eirage@chromium.org>
Date: Thu Nov 09 00:59:05 2017

Enable prevent scrolling in Element.focus()

Implement Focusptions for focus() under a experimental flag.
FocusOptions has a boolean preventScroll (default false) to
prevent scrolling.

spec: https://html.spec.whatwg.org/multipage/interaction.html#focus-management-apis
web-platform-tests:
https://github.com/w3c/web-platform-tests/pull/7915
https://github.com/w3c/web-platform-tests/pull/7917

Intent to implement and ship:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/55sC81ciQgY

Bug:  734166 
Change-Id: I47c1ebd9aebfc48064f9712d11c4d07ab8fabfa4
Reviewed-on: https://chromium-review.googlesource.com/731225
Reviewed-by: Rick Byers <rbyers@chromium.org>
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Reviewed-by: Navid Zolghadr <nzolghadr@chromium.org>
Commit-Queue: Ella Ge <eirage@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515040}
[delete] https://crrev.com/2352f5672ed01cdbdd6f9fecb3fdd1a625a9fbfc/third_party/WebKit/LayoutTests/external/wpt/html/editing/focus/processing-model/preventScroll-expected.txt
[modify] https://crrev.com/d4b0ae2e7c827f152f94c0886465954b18590af0/third_party/WebKit/LayoutTests/external/wpt/html/editing/focus/processing-model/preventScroll.html
[modify] https://crrev.com/d4b0ae2e7c827f152f94c0886465954b18590af0/third_party/WebKit/Source/core/core_idl_files.gni
[modify] https://crrev.com/d4b0ae2e7c827f152f94c0886465954b18590af0/third_party/WebKit/Source/core/dom/Document.cpp
[modify] https://crrev.com/d4b0ae2e7c827f152f94c0886465954b18590af0/third_party/WebKit/Source/core/dom/Element.cpp
[modify] https://crrev.com/d4b0ae2e7c827f152f94c0886465954b18590af0/third_party/WebKit/Source/core/dom/Element.h
[add] https://crrev.com/d4b0ae2e7c827f152f94c0886465954b18590af0/third_party/WebKit/Source/core/html/FocusOptions.idl
[modify] https://crrev.com/d4b0ae2e7c827f152f94c0886465954b18590af0/third_party/WebKit/Source/core/html/HTMLAreaElement.cpp
[modify] https://crrev.com/d4b0ae2e7c827f152f94c0886465954b18590af0/third_party/WebKit/Source/core/html/HTMLAreaElement.h
[modify] https://crrev.com/d4b0ae2e7c827f152f94c0886465954b18590af0/third_party/WebKit/Source/core/html/HTMLElement.idl
[modify] https://crrev.com/d4b0ae2e7c827f152f94c0886465954b18590af0/third_party/WebKit/Source/core/html/forms/HTMLInputElement.cpp
[modify] https://crrev.com/d4b0ae2e7c827f152f94c0886465954b18590af0/third_party/WebKit/Source/core/html/forms/HTMLInputElement.h
[modify] https://crrev.com/d4b0ae2e7c827f152f94c0886465954b18590af0/third_party/WebKit/Source/core/html/forms/HTMLLabelElement.cpp
[modify] https://crrev.com/d4b0ae2e7c827f152f94c0886465954b18590af0/third_party/WebKit/Source/core/html/forms/HTMLLegendElement.cpp
[modify] https://crrev.com/d4b0ae2e7c827f152f94c0886465954b18590af0/third_party/WebKit/Source/core/html/forms/HTMLTextAreaElement.cpp
[modify] https://crrev.com/d4b0ae2e7c827f152f94c0886465954b18590af0/third_party/WebKit/Source/core/html/forms/HTMLTextAreaElement.h
[modify] https://crrev.com/d4b0ae2e7c827f152f94c0886465954b18590af0/third_party/WebKit/Source/platform/runtime_enabled_features.json5

Labels: M-64
Status: Fixed (was: Assigned)
Project Member

Comment 15 by bugdroid1@chromium.org, Aug 7

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b7e2c229baf66c61b82c35a0145b04a5c829c575

commit b7e2c229baf66c61b82c35a0145b04a5c829c575
Author: Eric Willigers <ericwilligers@chromium.org>
Date: Tue Aug 07 06:51:09 2018

DOM: retire runtime flag for FocusOptions

Ability to Prevent scrolling in HTMLElement.focus() shipped in M64
https://chromium-review.googlesource.com/731225
https://www.chromestatus.com/feature/5745122025144320

BUG= 734166 

Change-Id: Ia2f8ebe29c553069e4893f7b474bd21b4b1f29a2
Reviewed-on: https://chromium-review.googlesource.com/1163056
Reviewed-by: Ella Ge <eirage@chromium.org>
Reviewed-by: Yoav Weiss <yoav@yoav.ws>
Commit-Queue: Eric Willigers <ericwilligers@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581154}
[modify] https://crrev.com/b7e2c229baf66c61b82c35a0145b04a5c829c575/third_party/blink/renderer/core/html/focus_options.idl
[modify] https://crrev.com/b7e2c229baf66c61b82c35a0145b04a5c829c575/third_party/blink/renderer/platform/runtime_enabled_features.json5

Sign in to add a comment