Remove default ScrollSourceType parameter value from SetScrollOffset [chromium/src : main]

0 views
Skip to first unread message

Munira Tursunova (Gerrit)

unread,
Sep 16, 2025, 2:32:34 PM (7 days ago) Sep 16
to Robert Flack, Chromium LUCI CQ, AyeAye, Alexis Menard, David Bokan, chromium...@chromium.org, Daniel Cheng, Olga Gerchikov, (Julie)Jeongeun Kim, Kevin Babbitt, Kenneth Rohde Christiansen, srirama chandra sekhar, abigailbk...@google.com, blink-rev...@chromium.org, blink-revie...@chromium.org, blink-re...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, dcheng+c...@chromium.org, dtapuska+...@chromium.org, dtseng...@chromium.org, eric.c...@apple.com, feature-me...@chromium.org, francisjp...@google.com, josiah...@chromium.org, kyungjunle...@google.com, lucasrada...@google.com, nektar...@chromium.org, web-schedulin...@chromium.org, yuzo+...@chromium.org, zol...@webkit.org
Attention needed from Robert Flack

New activity on the change

Open in Gerrit

Related details

Attention is currently required from:
  • Robert Flack
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement is not satisfiedCode-Owners
  • requirement is not satisfiedCode-Review
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: I0e26d54d09002ef32114ce466c82dae327b8b5be
Gerrit-Change-Number: 6930259
Gerrit-PatchSet: 6
Gerrit-Owner: Munira Tursunova <moo...@google.com>
Gerrit-Reviewer: Munira Tursunova <moo...@google.com>
Gerrit-Reviewer: Robert Flack <fla...@chromium.org>
Gerrit-CC: (Julie)Jeongeun Kim <je_jul...@chromium.org>
Gerrit-CC: Akihiro Ota <akihi...@chromium.org>
Gerrit-CC: Alexis Menard <alexis...@intel.com>
Gerrit-CC: Daniel Cheng <dch...@chromium.org>
Gerrit-CC: David Bokan <bo...@chromium.org>
Gerrit-CC: Kenneth Rohde Christiansen <kenneth.ch...@gmail.com>
Gerrit-CC: Kevin Babbitt <kbab...@microsoft.com>
Gerrit-CC: Nektarios Paisios <nek...@chromium.org>
Gerrit-CC: Olga Gerchikov <gerc...@microsoft.com>
Gerrit-CC: srirama chandra sekhar <srir...@samsung.com>
Gerrit-Attention: Robert Flack <fla...@chromium.org>
Gerrit-Comment-Date: Tue, 16 Sep 2025 18:32:16 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
satisfied_requirement
unsatisfied_requirement
open
diffy

Robert Flack (Gerrit)

unread,
Sep 19, 2025, 5:00:52 PM (4 days ago) Sep 19
to Munira Tursunova, David Awogbemila, Chromium LUCI CQ, AyeAye, Alexis Menard, David Bokan, chromium...@chromium.org, Daniel Cheng, Olga Gerchikov, (Julie)Jeongeun Kim, Kevin Babbitt, Kenneth Rohde Christiansen, srirama chandra sekhar, abigailbk...@google.com, blink-rev...@chromium.org, blink-revie...@chromium.org, blink-re...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, dcheng+c...@chromium.org, dtapuska+...@chromium.org, dtseng...@chromium.org, eric.c...@apple.com, feature-me...@chromium.org, francisjp...@google.com, josiah...@chromium.org, kyungjunle...@google.com, lucasrada...@google.com, nektar...@chromium.org, web-schedulin...@chromium.org, yuzo+...@chromium.org, zol...@webkit.org
Attention needed from David Awogbemila and Munira Tursunova

Robert Flack voted and added 8 comments

Votes added by Robert Flack

Code-Review+1

8 comments

Patchset-level comments
File-level comment, Patchset 7 (Latest):
Robert Flack . unresolved

I left a bunch of specific cases that I think could be given the correct type now.

Additionally, I think that nearly all existing unit test scrolls could be considered absolute.

Overall lgtm.

File third_party/blink/renderer/core/frame/root_frame_viewport.cc
Line 409, Patchset 7 (Latest): cc::ScrollSourceType::kNone);
Robert Flack . unresolved

Given this is part of a scroll into view request, it should be absolute right?

File third_party/blink/renderer/core/frame/visual_viewport.cc
Line 811, Patchset 7 (Latest): cc::ScrollSourceType::kNone, params->behavior,
Robert Flack . unresolved

Same here, as a scroll into view request i think this is absolute.

File third_party/blink/renderer/core/html/forms/text_field_input_type.cc
Line 278, Patchset 7 (Latest): cc::ScrollSourceType::kNone);
Robert Flack . unresolved

This seems like it would be an absolute scroll - since it's scrolling to the origin.

File third_party/blink/renderer/core/paint/paint_layer_scrollable_area.cc
Line 258, Patchset 7 (Latest): cc::ScrollSourceType::kNone,
Robert Flack . unresolved

This seems to be intended to be an anchoring stationary scroll.

Line 1250, Patchset 7 (Latest): cc::ScrollSourceType::kNone, mojom::blink::ScrollBehavior::kInstant,
Robert Flack . unresolved

This would be a stationary scroll.

Line 2508, Patchset 7 (Latest): cc::ScrollSourceType::kNone, behavior, ScrollCallback(),
Robert Flack . unresolved

scroll into view is an absolute scroll, right?

File third_party/blink/renderer/core/scroll/scrollable_area.cc
Line 555, Patchset 7 (Latest): cc::ScrollSourceType::kNone,
Robert Flack . unresolved

I suspect this should be an absolute scroll as it's similar to an anchor scroll.

Open in Gerrit

Related details

Attention is currently required from:
  • David Awogbemila
  • Munira Tursunova
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement satisfiedCode-Owners
  • requirement satisfiedCode-Review
  • requirement is not satisfiedNo-Unresolved-Comments
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: I0e26d54d09002ef32114ce466c82dae327b8b5be
Gerrit-Change-Number: 6930259
Gerrit-PatchSet: 7
Gerrit-Owner: Munira Tursunova <moo...@google.com>
Gerrit-Reviewer: David Awogbemila <awogb...@chromium.org>
Gerrit-Reviewer: Munira Tursunova <moo...@google.com>
Gerrit-Reviewer: Robert Flack <fla...@chromium.org>
Gerrit-CC: (Julie)Jeongeun Kim <je_jul...@chromium.org>
Gerrit-CC: Akihiro Ota <akihi...@chromium.org>
Gerrit-CC: Alexis Menard <alexis...@intel.com>
Gerrit-CC: Daniel Cheng <dch...@chromium.org>
Gerrit-CC: David Bokan <bo...@chromium.org>
Gerrit-CC: Kenneth Rohde Christiansen <kenneth.ch...@gmail.com>
Gerrit-CC: Kevin Babbitt <kbab...@microsoft.com>
Gerrit-CC: Nektarios Paisios <nek...@chromium.org>
Gerrit-CC: Olga Gerchikov <gerc...@microsoft.com>
Gerrit-CC: srirama chandra sekhar <srir...@samsung.com>
Gerrit-Attention: David Awogbemila <awogb...@chromium.org>
Gerrit-Attention: Munira Tursunova <moo...@google.com>
Gerrit-Comment-Date: Fri, 19 Sep 2025 21:00:40 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
satisfied_requirement
unsatisfied_requirement
open
diffy

Munira Tursunova (Gerrit)

unread,
Sep 22, 2025, 5:36:15 PM (17 hours ago) Sep 22
to Robert Flack, David Awogbemila, Chromium LUCI CQ, AyeAye, Alexis Menard, David Bokan, chromium...@chromium.org, Daniel Cheng, Olga Gerchikov, (Julie)Jeongeun Kim, Kevin Babbitt, Kenneth Rohde Christiansen, srirama chandra sekhar, abigailbk...@google.com, blink-rev...@chromium.org, blink-revie...@chromium.org, blink-re...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, dcheng+c...@chromium.org, dtapuska+...@chromium.org, dtseng...@chromium.org, eric.c...@apple.com, feature-me...@chromium.org, francisjp...@google.com, josiah...@chromium.org, kyungjunle...@google.com, lucasrada...@google.com, nektar...@chromium.org, web-schedulin...@chromium.org, yuzo+...@chromium.org, zol...@webkit.org
Attention needed from David Awogbemila and Robert Flack

Munira Tursunova added 8 comments

Patchset-level comments
Robert Flack . unresolved

I left a bunch of specific cases that I think could be given the correct type now.

Additionally, I think that nearly all existing unit test scrolls could be considered absolute.

Overall lgtm.

Munira Tursunova

Some of them, like scroll_anchor_test.cc, I assume should be stationary scrolls, but since none of them test scroll direction and scroll type should not affect those tests now, I didn't see the point to go through all and put the correct scroll type there, so I just put `ScrollType::kNone` in almost all of them.

File third_party/blink/renderer/core/frame/root_frame_viewport.cc
Line 409, Patchset 7: cc::ScrollSourceType::kNone);
Robert Flack . resolved

Given this is part of a scroll into view request, it should be absolute right?

Munira Tursunova

It doesn't explicitly say so in spec, but sounds like it should be. Changed, thanks.

File third_party/blink/renderer/core/frame/visual_viewport.cc
Line 811, Patchset 7: cc::ScrollSourceType::kNone, params->behavior,
Robert Flack . resolved

Same here, as a scroll into view request i think this is absolute.

Munira Tursunova

Done

File third_party/blink/renderer/core/html/forms/text_field_input_type.cc
Line 278, Patchset 7: cc::ScrollSourceType::kNone);
Robert Flack . resolved

This seems like it would be an absolute scroll - since it's scrolling to the origin.

Munira Tursunova

Done

File third_party/blink/renderer/core/paint/paint_layer_scrollable_area.cc
Line 258, Patchset 7: cc::ScrollSourceType::kNone,
Robert Flack . resolved

This seems to be intended to be an anchoring stationary scroll.

Munira Tursunova

Done

Line 1250, Patchset 7: cc::ScrollSourceType::kNone, mojom::blink::ScrollBehavior::kInstant,
Robert Flack . resolved

This would be a stationary scroll.

Munira Tursunova

Done

Line 2508, Patchset 7: cc::ScrollSourceType::kNone, behavior, ScrollCallback(),
Robert Flack . resolved

scroll into view is an absolute scroll, right?

Munira Tursunova

It doesn't explicitly say so in spec, but sounds like it should be. Changed, thanks.

File third_party/blink/renderer/core/scroll/scrollable_area.cc
Line 555, Patchset 7: cc::ScrollSourceType::kNone,
Robert Flack . resolved

I suspect this should be an absolute scroll as it's similar to an anchor scroll.

Munira Tursunova

You mentioned before that kScrollStart scrolls are none scrolls? https://chromium-review.googlesource.com/c/chromium/src/+/6798626/comment/779f8b8a_6f9813a6/

Open in Gerrit

Related details

Attention is currently required from:
  • David Awogbemila
  • Robert Flack
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement satisfiedCode-Owners
  • requirement satisfiedCode-Review
  • requirement is not satisfiedNo-Unresolved-Comments
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: I0e26d54d09002ef32114ce466c82dae327b8b5be
Gerrit-Change-Number: 6930259
Gerrit-PatchSet: 9
Gerrit-Owner: Munira Tursunova <moo...@google.com>
Gerrit-Reviewer: David Awogbemila <awogb...@chromium.org>
Gerrit-Reviewer: Munira Tursunova <moo...@google.com>
Gerrit-Reviewer: Robert Flack <fla...@chromium.org>
Gerrit-CC: (Julie)Jeongeun Kim <je_jul...@chromium.org>
Gerrit-CC: Akihiro Ota <akihi...@chromium.org>
Gerrit-CC: Alexis Menard <alexis...@intel.com>
Gerrit-CC: Daniel Cheng <dch...@chromium.org>
Gerrit-CC: David Bokan <bo...@chromium.org>
Gerrit-CC: Kenneth Rohde Christiansen <kenneth.ch...@gmail.com>
Gerrit-CC: Kevin Babbitt <kbab...@microsoft.com>
Gerrit-CC: Nektarios Paisios <nek...@chromium.org>
Gerrit-CC: Olga Gerchikov <gerc...@microsoft.com>
Gerrit-CC: srirama chandra sekhar <srir...@samsung.com>
Gerrit-Attention: David Awogbemila <awogb...@chromium.org>
Gerrit-Attention: Robert Flack <fla...@chromium.org>
Gerrit-Comment-Date: Mon, 22 Sep 2025 21:35:59 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Robert Flack <fla...@chromium.org>
satisfied_requirement
unsatisfied_requirement
open
diffy
Reply all
Reply to author
Forward
0 new messages