markbind icon indicating copy to clipboard operation
markbind copied to clipboard

Scroll to anchor behavior is different when a anchor link is opened in a different tab

Open damithc opened this issue 4 years ago • 12 comments

v2.12.0, Chrome, Windows

Ctrl+Click (to open in new tab) on this link

Expected (this is what you get if you normal-click on the link): image

Actual: image

damithc avatar Mar 26 '20 02:03 damithc

@le0tan can take a look?

damithc avatar Mar 26 '20 02:03 damithc

@damithc I can't reproduce this bug. Is this bug still there after restarting the browser?

le0tan avatar Mar 26 '20 02:03 le0tan

@le0tan Noticed this in two computers. Yes, can reproduce. May be specific to OS and Browser?

damithc avatar Mar 26 '20 02:03 damithc

@damithc I'm on Windows as well, which version of Chrome are you using?

le0tan avatar Mar 26 '20 02:03 le0tan

Tried on Edge. Sometimes work, sometimes end up in at the totally wrong place. image

damithc avatar Mar 26 '20 02:03 damithc

I now can reproduce this using Firefox. I can confirm that the anchors are correctly generated and scrollToUrlAnchorHeading is working correctly (i.e. it shouldn't be caused by the fixed navbar feature). Probably has something to do with the order of executing the scripts, investigating.

le0tan avatar Mar 26 '20 02:03 le0tan

Could it be due to expanded panels?

There are quite a few expanded panels in the 2113 website.

I faced the same problem on the 2113 website on Ubuntu 18.04 Firefox.

But I tried on

https://markbind.org/userGuide/fullSyntaxReference.html#lists

and it always scrolled to the correct heading.

openorclose avatar Mar 28 '20 09:03 openorclose

Could it be due to expanded panels?

There are quite a few expanded panels in the 2113 website.

I faced the same problem on the 2113 website on Ubuntu 18.04 Firefox.

But I tried on

https://markbind.org/userGuide/fullSyntaxReference.html#lists

and it always scrolled to the correct heading.

Not likely as I tried on the MarkBind doc, and there's no expanded panels in the documentation.

le0tan avatar Mar 28 '20 12:03 le0tan

Just encountered this. My finding is that if the anchor link is

  • left-click and view on the same page -> the scroll works as expected
  • right-click and open on a new tab, and immediately click into that tab before it has finished rendering -> the scroll works as expected (not sure if this is the always true but looks like it)

The error occurs when you

  • right-click and open on a new tab, wait for a few seconds and click into that tab -> the scroll covers the heading as shown in the original post.

Reproduction step:

  • https://markbind-master.netlify.app/userguide/formattingcontents#code
  • Click on "here", which will link you to the "style" section on https://markbind-master.netlify.app/userguide/sitejsonfile#style nothing is wrong
  • right-click on "here" and open a new tab, wait a few seconds until the new page has finished rendering, and check that page to see that the scroll is wrong.

Recording: 5EtHcUVINj

Not sure but it might be related to #1948

tlylt avatar Jun 23 '22 14:06 tlylt

could it simply be due to the ordering? (should be swapped) (still, odd that it works using a "normal" click to open)

  scrollToUrlAnchorHeading();
  detectAndApplyHeaderStyles();

I think we could try adapting this part to use the detected sticky header height as well, if any

  window.scrollBy(0, -document.body.style.paddingTop.replace('px', ''));

ang-zeyu avatar Jun 25 '22 08:06 ang-zeyu

I did some research on this. This issue should only be attempted after #1953 is merged.

The issue with this is that scrollIntoView does not work when the document is not visible on our end.

Some solutions that I have attempted but to no avail.

  1. One time event listen to visibility change. On visbility change to visible, run scrollIntoView. 1a. This work but somehow scroll margin top is not of the correct value on switch to new tab. So, the nav bar will cover the header.
  2. Also attempted to scroll to position with offset. 2a. Same problem as 1a.

Somehow, even if we perform scrolling on visible and after onLoad, the scroll margin top value is still incorrect initially.

I only have hacky ideas to fix it.

  1. We can follow detectAndApplyHeaderStyles in the same file as scrollToUrlAnchorHeading to retrieve --sticky-header-height which can be used to scroll it correctly with scroll to position with offset. 1a. Potential issue would be that what if we have some element that has a scroll-margin-top that is not equal to --sticky-header-height.
  2. Through observer or some event loop, to check for scroll margin top and only scroll when the value is updated.

EltonGohJH avatar Mar 26 '23 13:03 EltonGohJH

In addition,

  • Scroll-to-anchor is broken on mobile (physical android) chrome when opening in new tab irregardless of how fast I switch to the new tab (or maybe I'm just not quick enough.. 🤷‍♂️).

    retrieve --sticky-header-height which can be used to scroll it correctly with scroll to position with offset.

    The offset solution also does not work in this case.

  • Consistently fine on Safari (Mac) but maybe I haven't tried enough times.

ang-zeyu avatar Apr 27 '23 08:04 ang-zeyu