Maniphest T11997

When following an anchor link, Chrome 55 no longer scrolls to elements which are completely hidden
Closed, ResolvedPublic
Actions

Assigned To

None

Authored By

	Volans
	Dec 12 2016, 10:11 AM

Description

Reproduction steps

Opening a link to a specific message of a Task (of the form /T12345#123456) doesn't scroll anymore to the anchor of the message on Chrome 55.
I was able to reproduce this both on Mac OS and Linux. On Linux I had Chrome 54 and it was working fine but after upgrading to Chrome 55 it stopped working.
Link to anchors on other websites that I've tried are still working as usual on Chrome 55, hence I'm assuming that this is an issue related to Phabricator and not Chrome 55, but it might be instead an issue with Chrome itself.

See for example a link to a task message in this installation too: T11953#202672

Version information: rP9c38b61e51a053f8aafe96bcc6b99291a1fe4bd3

Revisions and Commits

rP Phabricator
	D17033	rP3277e7873211 Move timeline anchor targets up to evade a Chrome 55 behavioral change

Related Objects

Mentioned In: T12779: File anchor links can load scrolled to the wrong file
T12032: Make sure Chrome 55 anchor issue does not impact Differential
Mentioned Here: D17033: Move timeline anchor targets up to evade a Chrome 55 behavioral change
T11998: Content flashes when loading pages in Chrome
rP9c38b61e51a0: Fix an issue where tokenizers can sort milestone results into the wrong query…
T11953: Herald rule for commits on autoclose branch didn't trigger when a commit was first pushed to a branch, then to master (fast-forward)

Event Timeline

Volans created this task.Dec 12 2016, 10:11 AM

chad added a subscriber: chad.Dec 12 2016, 5:15 PM

This comment was removed by chad.

This doesn't reproduce for me on this install in Chrome 55/Mac, or any other browser. I'm assuming the issue you saw on this server was T11998 which was a local configuration issue.

Do you have a set of steps I can reproduce locally if you're seeing this on other installs?

This appears to be a bug in Chrome, because this fixes it:

diff --git a/webroot/rsrc/css/application/base/standard-page-view.css b/webroot/rsrc/css/application/base/standard-page-view.css
index ebcf157ad..d48d29869 100644
--- a/webroot/rsrc/css/application/base/standard-page-view.css
+++ b/webroot/rsrc/css/application/base/standard-page-view.css
@@ -5,7 +5,6 @@
 .phabricator-anchor-view,
 .phabricator-anchor-navigation-marker {
   position: absolute;
-  margin-top: -15px;
 }
 
 .phabricator-chromeless-page .phabricator-standard-page {

I'll attempt to narrow this down and report it upstream.

Oh, it reproduces for me.

Basically, I've found:

Works:
Mac / Safari
Mac / Firefox
Mac / Chrome
Win / Firefox
Ubuntu / Firefox

Doesn't
Win / IE
Win / Chrome
Ubuntu / Chrome

@chad I was replying but saw now your last comments, thanks @epriestley for narrowing it down. Interesting that I can repro on Chrome 55/Mac while you can't.

I reported it because I saw it in Wikimedia installation, for example in https://phabricator.wikimedia.org/T144431#2858030 but any direct link to a task's post works the same.

I reported it here because all the other websites that have anchored links that I tried were working fine, so it might be related to the way Phabricator is handling the anchors, but that doesn't exclude an upstream Chrome bug, as it seems from the lasts comments.

Hah, I was on Chrome 54, and my report above works. But after updating to 55, it no longer works. Something changed in Chrome. Not sure what though.

Here's my reduced test case for Chome 55:

<!DOCTYPE html>
<html>
  <body>
    <div style="height: 50em;"></div>

    <a href="#target1">Jump to Hidden Target</a>
    |
    <a href="#target2">Jump to Visible Target</a>

    <div style="height: 25em;"></div>

    <div style="position: relative; overflow: hidden;">
      <a name="target1" style="position: absolute; margin-top: -15px; height: 15px;"></a>
      <a name="target2" style="position: absolute; margin-top: -15px; height: 16px;"></a>
      Target
    </div>

    <div style="height: 50em;"></div>
  </body>
</html>

Basically, if an anchor target is completely hidden, Chrome will no longer jump to it.

Let me ship that report upstream (it's inconsistent with other browsers, at a minimum) and then I'll look at fixing this.

@epriestley could it be related to this change? https://chromium.googlesource.com/chromium/src/+/a67debf9a9b78f60e7b10b5574ae4ceab49af31b%5E%21/#F2

I was looking at changes between the last 2 versions but there are a tons...
https://chromium.googlesource.com/chromium/src/+log/54.0.2840.98..55.0.2883.75?pretty=fuller&n=10000

@chad was that for me or for Chrome? 😄

Chromez

Here's my upstream report:

https://bugs.chromium.org/p/chromium/issues/detail?id=673396

A workaround is to position or size the <a name="..."> element so that any part of it is not hidden by an overflow: hidden; container.

See also https://code.google.com/p/chromium/issues/detail?id=473657 for earlier adventures in Chrome.

I'm going to see if we can move the anchor outside the container. If not, we can probably simply make it more than 15px tall, since it's an invisible element with no width anyway. The goal of the -15px is to scroll the browser window to a little above the link target, since this is more pleasant and readable than the default behavior where the browser scrolls the link target to the very top of the screen.

epriestley renamed this task from Links to Task's messages don't scroll to the message on Chrome 55 to Chrome 55 no longer scrolls to elements which are completely hidden.Dec 12 2016, 6:16 PM

epriestley renamed this task from Chrome 55 no longer scrolls to elements which are completely hidden to When following an anchor link, Chrome 55 no longer scrolls to elements which are completely hidden.

epriestley added a revision: D17033: Move timeline anchor targets up to evade a Chrome 55 behavioral change.Dec 12 2016, 6:36 PM

epriestley closed this task as Resolved by committing rP3277e7873211: Move timeline anchor targets up to evade a Chrome 55 behavioral change.Dec 12 2016, 6:39 PM

epriestley added a commit: rP3277e7873211: Move timeline anchor targets up to evade a Chrome 55 behavioral change.

That Chrome change seems at least plausible as a root cause to me, but I definitely don't know enough about the Chrome internals to really hazard a guess.

D17033 moved the anchor up, outside the div with overflow: hidden;. That appeared to fix things for me locally.

I've deployed the change to this server if you want to test if it's resolved for you (seems OK to me).

Thanks for the report! Let us know if you run into anything else.

@epriestley I can confirm it works fine both on Mac and Linux for me. Thanks a lot for the finding and quick fix!

epriestley mentioned this in T12032: Make sure Chrome 55 anchor issue does not impact Differential.Dec 19 2016, 9:35 PM

Chrome upstream has patched this, now:

https://chromium.googlesource.com/chromium/src.git/+/961e2d024412588edffbef5b3f6dfdfd2646761f

alexmv mentioned this in T12779: File anchor links can load scrolled to the wrong file.May 30 2017, 11:43 PM

	F2112173: 2392289.png
	Dec 12 2016, 6:06 PM