brave / brave-browser

Brave browser for Android, iOS, Linux, macOS, Windows.
https://brave.com
Mozilla Public License 2.0
17.87k stars 2.34k forks source link

location.hash is empty string for "URL Fragment Text Directives" type links, yet feature itself is not implemented #37041

Open stopsopa opened 7 months ago

stopsopa commented 7 months ago

Description

Navigation to "URL Fragment Text Directives" seems to not be implemented in Brave yet making locaction.hash = "" still happens in Brave. This causes problems with polyfilling this functionality.

Steps to Reproduce

  1. load the page https://text-fragments-polyfill.glitch.me/#:~:text=This%20domain,examples&text=More%20information...
  2. and then open developers tools and put breakpoint on the next line after line with location.hash and reload the page image

or

  1. load the page https://text-fragments-polyfill.glitch.me/#:~:text=This%20domain,examples&text=More%20information...
  2. and run in developer tools console.log(location.hash)

Actual result:

After thorough debugging I've discovered that for some reason when hash part of url contains #:~:text= (so called "URL Fragment Text Directives" also described in MDN) then Brave is hiding entire hash.

Expected result:

Normally it should contain the hash. Can be tested also directly in console with console.log(location.hash) in developers tools. On the page https://text-fragments-polyfill.glitch.me/#:~:text=This%20domain,examples&text=More%20information... It should be equal to:

console.log(location.hash)
'#:~:text=This%20domain,examples&text=in%20literature&text=More%20information...'

or

scrolling to "URL Fragment Text Directives" should be implemented then hash could be considered "consumend" then making location.hash = '' would be justified. As it is the case in chrome.

Reproduces how often:

always

Brave version (brave://version info)

[Version 1.63.169 Chromium: 122.0.6261.111 (Official Build) (arm64)]

Version/Channel Information:

Other Additional Information:

Miscellaneous Information:

Generally it seems hash is accessible as long as it is not #:~:text= version

It seems that chrome (not Brave) is "consuming" #:~:text= which means it navigates to the fragment on the page and then it makes sense to emptystringify location.hash to prevent further processing by js scripts on the page.

The problem is that generally since scrolling to "URL Fragment Text Directives" is not implemented in Brave then why block creating polyfill with emptystringing location.hash?

That's why it falls to the category of Brave issues not chrome.

This actually creates functionality gap impossible to mitigate - And that's exactly what I'm trying to do here -> create tampermonkey based on polyfill just for me as a temporary fix of that feature. Because I believe at some point it will be enabled in Brave too.. right?

I can see two solutions for now:

I know that it sounds like nitpicking but "URL Fragment Text Directives" seems to be more and more in use these days obviously because Google making it popular returning links with such fragments and also making it work in chrome.

diracdeltas commented 7 months ago

is this an intentional feature? @ShivanKaul @fmarier in case you have any idea

ShivanKaul commented 7 months ago

We currently disable the Scroll to Text Fragment feature for privacy reasons, but with plans for re-enablement.

stopsopa commented 7 months ago

But then providing toggle or not masking location.hash as an empty string would be nice.

With all respect - I love Brave. But what you guys did here disabling this feature sucks. Not only feature is disabled but can't be mitigated in any way due to the way it was disabled.

If one really have something to do which requires those links to work then ... sorry, just use different browser.

eligrey commented 5 months ago

@ShivanKaul Your statement is not correct. Brave currently enables part of the Scroll to Text Fragment feature due to insufficiently scoped gating logic, which is the cause of this issue in the first place.

If I am mistaken, please link to documentation explaining this 'strip out all fragment directives from the URL' feature that Brave uniquely implements.