Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Disable ScrollToTextFragment #8342

Closed
jumde opened this issue Feb 21, 2020 · 7 comments · Fixed by brave/brave-core#4548
Closed

Disable ScrollToTextFragment #8342

jumde opened this issue Feb 21, 2020 · 7 comments · Fixed by brave/brave-core#4548
Assignees
@jumde
Labels
priority/P3 The next thing for us to work on. It'll ride the trains. privacy/chromium-redqueen Work to remove or improve privacy-harming "features" added in Chromium. privacy/scroll-to-text-fragment privacy QA Pass-Linux QA Pass-macOS QA Pass-Win64 QA/Yes release-notes/include
Milestone
1.8.x - Release

Comments

@jumde
Copy link
Contributor

jumde commented Feb 21, 2020 •
edited
Loading

https://www.theregister.co.uk/2020/02/20/chrome_deploys_deeplinking/

Test Plan

  1. Navigate to https://example.com/#:~:text=domain
  2. Text Domain should not be highlighted. This works on chrome.
@jumde jumde self-assigned this Feb 21, 2020
@tildelowengrimm tildelowengrimm added priority/P3 The next thing for us to work on. It'll ride the trains. privacy/chromium-redqueen Work to remove or improve privacy-harming "features" added in Chromium. labels Feb 24, 2020
@jumde
Copy link
Contributor Author

jumde commented Feb 25, 2020

https://chromium.googlesource.com/chromium/src/+/62f8d5f096b174d05cd7f716585983df483024d7%5E%21/#F0 - This is enabled in Chromium using Finch, will be enabled by default in M81.

mkarolin added a commit to brave/brave-core that referenced this issue Feb 28, 2020
@mkarolin
Disables blink::features::kTextFragmentAnchor
18e149c 
Fixes brave/brave-browser#8342

Chromium change:

https://chromium.googlesource.com/chromium/src/+/62f8d5f096b174d05cd7f716585983df483024d7

commit 62f8d5f096b174d05cd7f716585983df483024d7
Author: Nick Burris <nburris@chromium.org>
Date:   Thu Jan 16 22:54:51 2020 +0000

    Enable Scroll To Text by default

    Enables scroll to text by default for M81. Note we plan to launch via
    Finch for M80.

    Intent to ship with LGTMs:
    https://groups.google.com/a/chromium.org/d/msg/blink-dev/zlLSxQ9BA8Y/t-_3pAiSAwAJ

    This patch also updates web platform test expectations:
    - scroll-to-text-fragment.html now passes; it just needed an extra rAF
      for the fallback to element anchor behavior.
    - scroll-to-text-fragment-security.html has two expected failures, as
      they need to be updated to test cross-origin navigations. I'll make
      this change in a follow-up patch to close bug 1042311.

    Bug: 919204
mkarolin added a commit to brave/brave-core that referenced this issue Mar 2, 2020
@mkarolin
Disables blink::features::kTextFragmentAnchor
4384725 
Fixes brave/brave-browser#8342

Chromium change:

https://chromium.googlesource.com/chromium/src/+/62f8d5f096b174d05cd7f716585983df483024d7

commit 62f8d5f096b174d05cd7f716585983df483024d7
Author: Nick Burris <nburris@chromium.org>
Date:   Thu Jan 16 22:54:51 2020 +0000

    Enable Scroll To Text by default

    Enables scroll to text by default for M81. Note we plan to launch via
    Finch for M80.

    Intent to ship with LGTMs:
    https://groups.google.com/a/chromium.org/d/msg/blink-dev/zlLSxQ9BA8Y/t-_3pAiSAwAJ

    This patch also updates web platform test expectations:
    - scroll-to-text-fragment.html now passes; it just needed an extra rAF
      for the fallback to element anchor behavior.
    - scroll-to-text-fragment-security.html has two expected failures, as
      they need to be updated to test cross-origin navigations. I'll make
      this change in a follow-up patch to close bug 1042311.

    Bug: 919204
mkarolin added a commit to brave/brave-core that referenced this issue Mar 4, 2020
@mkarolin
Disables blink::features::kTextFragmentAnchor
91f1780 
Fixes brave/brave-browser#8342

Chromium change:

https://chromium.googlesource.com/chromium/src/+/62f8d5f096b174d05cd7f716585983df483024d7

commit 62f8d5f096b174d05cd7f716585983df483024d7
Author: Nick Burris <nburris@chromium.org>
Date:   Thu Jan 16 22:54:51 2020 +0000

    Enable Scroll To Text by default

    Enables scroll to text by default for M81. Note we plan to launch via
    Finch for M80.

    Intent to ship with LGTMs:
    https://groups.google.com/a/chromium.org/d/msg/blink-dev/zlLSxQ9BA8Y/t-_3pAiSAwAJ

    This patch also updates web platform test expectations:
    - scroll-to-text-fragment.html now passes; it just needed an extra rAF
      for the fallback to element anchor behavior.
    - scroll-to-text-fragment-security.html has two expected failures, as
      they need to be updated to test cross-origin navigations. I'll make
      this change in a follow-up patch to close bug 1042311.

    Bug: 919204
@mkarolin mkarolin mentioned this issue Mar 12, 2020
Merged
32 tasks
mkarolin added a commit to brave/brave-core that referenced this issue Mar 12, 2020
@mkarolin
Disables blink::features::kTextFragmentAnchor
e106728 
Fixes brave/brave-browser#8342

Chromium change:

https://chromium.googlesource.com/chromium/src/+/62f8d5f096b174d05cd7f716585983df483024d7

commit 62f8d5f096b174d05cd7f716585983df483024d7
Author: Nick Burris <nburris@chromium.org>
Date:   Thu Jan 16 22:54:51 2020 +0000

    Enable Scroll To Text by default

    Enables scroll to text by default for M81. Note we plan to launch via
    Finch for M80.

    Intent to ship with LGTMs:
    https://groups.google.com/a/chromium.org/d/msg/blink-dev/zlLSxQ9BA8Y/t-_3pAiSAwAJ

    This patch also updates web platform test expectations:
    - scroll-to-text-fragment.html now passes; it just needed an extra rAF
      for the fallback to element anchor behavior.
    - scroll-to-text-fragment-security.html has two expected failures, as
      they need to be updated to test cross-origin navigations. I'll make
      this change in a follow-up patch to close bug 1042311.

    Bug: 919204
@bsclifton bsclifton added this to the 1.8.x - Nightly milestone Mar 13, 2020
mkarolin added a commit to brave/brave-core that referenced this issue Mar 17, 2020
@mkarolin
Disables blink::features::kTextFragmentAnchor
b62a61f 
Fixes brave/brave-browser#8342

Chromium change:

https://chromium.googlesource.com/chromium/src/+/62f8d5f096b174d05cd7f716585983df483024d7

commit 62f8d5f096b174d05cd7f716585983df483024d7
Author: Nick Burris <nburris@chromium.org>
Date:   Thu Jan 16 22:54:51 2020 +0000

    Enable Scroll To Text by default

    Enables scroll to text by default for M81. Note we plan to launch via
    Finch for M80.

    Intent to ship with LGTMs:
    https://groups.google.com/a/chromium.org/d/msg/blink-dev/zlLSxQ9BA8Y/t-_3pAiSAwAJ

    This patch also updates web platform test expectations:
    - scroll-to-text-fragment.html now passes; it just needed an extra rAF
      for the fallback to element anchor behavior.
    - scroll-to-text-fragment-security.html has two expected failures, as
      they need to be updated to test cross-origin navigations. I'll make
      this change in a follow-up patch to close bug 1042311.

    Bug: 919204
mkarolin added a commit to brave/brave-core that referenced this issue Mar 19, 2020
@mkarolin
Disables blink::features::kTextFragmentAnchor
3221538 
Fixes brave/brave-browser#8342

Chromium change:

https://chromium.googlesource.com/chromium/src/+/62f8d5f096b174d05cd7f716585983df483024d7

commit 62f8d5f096b174d05cd7f716585983df483024d7
Author: Nick Burris <nburris@chromium.org>
Date:   Thu Jan 16 22:54:51 2020 +0000

    Enable Scroll To Text by default

    Enables scroll to text by default for M81. Note we plan to launch via
    Finch for M80.

    Intent to ship with LGTMs:
    https://groups.google.com/a/chromium.org/d/msg/blink-dev/zlLSxQ9BA8Y/t-_3pAiSAwAJ

    This patch also updates web platform test expectations:
    - scroll-to-text-fragment.html now passes; it just needed an extra rAF
      for the fallback to element anchor behavior.
    - scroll-to-text-fragment-security.html has two expected failures, as
      they need to be updated to test cross-origin navigations. I'll make
      this change in a follow-up patch to close bug 1042311.

    Bug: 919204
@mkarolin mkarolin mentioned this issue Apr 8, 2020
Merged
32 tasks
@LaurenWags
Copy link
Member

@jumde if this issue requires manual QA please add QA/Yes label and a test plan. If not, please add QA/No label. Thanks! cc @kjozwiak

@jumde
Copy link
Contributor Author

jumde commented Apr 9, 2020

@LaurenWags - added the test plan to the description, let me know if you need more details.

@jumde jumde added the QA/Yes label Apr 9, 2020
@LaurenWags
Copy link
Member

LaurenWags commented Apr 10, 2020 •
edited by GeetaSarvadnya
Loading

Verified passed with

Brave 1.8.71 Chromium: 81.0.4044.92 (Official Build) dev (64-bit)
Revision e98e6f21168a55e7ba57202f56323911cd9d31d1-refs/branch-heads/4044@{#883}
OS macOS Version 10.14.6 (Build 18G3020)
  • Verified test plan from the description

Screen Shot 2020-04-10 at 11 27 34 AM

  • Confirmed test plan from description does work in Chrome (81.0.4044.92) as noted.

Verification passed on

Brave 1.8.72 Chromium: 81.0.4044.92 (Official Build) dev (64-bit)
Revision e98e6f21168a55e7ba57202f56323911cd9d31d1-refs/branch-heads/4044@{#883}
OS Ubuntu 18.04 LTS
  • Verified test plan from the description
    image

Verification passed on

Brave 1.8.77 Chromium: 81.0.4044.92 (Official Build) dev (64-bit)
Revision e98e6f21168a55e7ba57202f56323911cd9d31d1-refs/branch-heads/4044@{#883}
OS Windows 10 OS Version 1803 (Build 17134.1006)
  • Verified the test plan from the description
    image

@LaurenWags LaurenWags mentioned this issue Apr 28, 2020
Closed
@dylan-k
Copy link

dylan-k commented Nov 30, 2021 •
edited
Loading

but why? This is a useful feature and its risks are (could be?) avoidable. Could it be optional at least?

@tomayac tomayac mentioned this issue Apr 29, 2022
Closed
@DutchPete
Copy link

Hey guys, disabling this feature without any recourse is not acceptable. I use this a lot so could you please at least make it togglable. It should be a user's choice, a user's decision whether to enable this or not.

@DutchPete DutchPete mentioned this issue May 1, 2022
Closed
@ShivanKaul ShivanKaul mentioned this issue Mar 26, 2024
Open
@pirate pirate mentioned this issue Nov 20, 2024
Open
@pirate
Copy link

pirate commented Nov 20, 2024

I've opened a new issue to formally request they re-add this flag: #42393

Please comment/vote over there as comments on closed PRs and issues likely wont get seen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jumde jumde

Labels
priority/P3 The next thing for us to work on. It'll ride the trains. privacy/chromium-redqueen Work to remove or improve privacy-harming "features" added in Chromium. privacy/scroll-to-text-fragment privacy QA Pass-Linux QA Pass-macOS QA Pass-Win64 QA/Yes release-notes/include
Projects
None yet
Milestone
1.8.x - Release
Development

Successfully merging a pull request may close this issue.

Upgrade from Chromium 80.0.3987.149 to Chromium 81.0.4044.69. brave/brave-core
10 participants
@pirate @tildelowengrimm @jumde @dylan-k @bsclifton @ShivanKaul @DutchPete @LaurenWags @btlechowski @GeetaSarvadnya