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.

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

Using csshyphens test breaks anchors on page load #1781

Closed
alexbaulch opened this issue Nov 30, 2015 · 9 comments · Fixed by #2369
Closed

Using csshyphens test breaks anchors on page load #1781

alexbaulch opened this issue Nov 30, 2015 · 9 comments · Fixed by #2369
Assignees

Comments

@alexbaulch
Copy link

Steps to reproduce:
Load a page using the Modernizr csshyphens test with an anchor/hash in the URL e.g. https://github.com/#some-anchor-link

Observed behaviour:
Page initially moves to desired element but then jumps back up to the top of the page

Desired behaviour:
Anchors should be respected on page load.

Notes:
It took me an age to get to the bottom of this and I was concerned that I might not be correct about the cause of the issue so I created a standalone test. Below are links to two basic HTML files that reference a full development build of Modernizr with and without the csshyphens test. The one with csshyphens test creates the issue described above. The one without works as expected.

With hyphens:
http://alexbaulch.com/modernizr-hyphens-issue/modernizr-with-hyphens.html#section-3

Without hyphens
http://alexbaulch.com/modernizr-hyphens-issue/modernizr-without-hyphens.html#section-3

@ryanseddon
Copy link
Member

@alexbaulch thanks for the detective work, think you can put together a PR to fix it?

@nibushibu
Copy link

I have same issue.

@BenjaminBeck
Copy link

same issue here, the page jumps to the top after executing the modernizr-script .
this also happens if you use the browser-back or reload button.
this is a big negative impact on usability for any site using this.

@bs-thomas
Copy link

I have the same issue.

@Sysix
Copy link

Sysix commented Jul 5, 2017

Any updates?

@usb248
Copy link

usb248 commented Jun 19, 2018

same issue

@rejas
Copy link
Member

rejas commented Jul 2, 2018

Debugged a little and it seems to me that those two lines are the culprits:

result = window.find(testword + testword);

Not sure how to replace the code or make it better, PRs are welcome :-)

@rejas
Copy link
Member

rejas commented Jul 2, 2018

PR was already there: #1633
Has to be tested though

@theLine
Copy link
Contributor

theLine commented Aug 26, 2018

Hi, I just purposed an other PR #2369 to this issue.
I think that saving the scroll position and restoring it, like in PR #1633, isn't a nice solution, because it won't really prevent the jumping (on really fast machines at least).

Unfortunately I wasn't able to really test it on many browsers/machines.

What do you think of this?

rejas pushed a commit that referenced this issue Oct 23, 2019
Positioned the created elements fixed to the top, to prevent any
jumping when (re-)loading the page.
When those elements weren't positioned fixed, they will be rendered at
the top of the document. In the test it's required to reset the
selection by focusing to a dummy input element thus triggering the
jump of the browser's viewport.

Successfully tested in the following browsers
* Chrome 68.0.3440.106 on MacOS 10.13.5
* Firefox 45.0.1 on MacOS 10.13.5
* Firefox 57.0.4 on MacOS 10.13.5
* Firefox 61.0.2 on MacOS 10.13.5
* Safari 11.1.1 (13605.2.8) on MacOS 10.13.5

* Chrome 68.0.3440.106 on Windows 10 Pro Build 1803
* Firefox 61.0.2 on Windows 10 Pro Build 1803
* Internet Explorer 11.228.17134.0 on Windows 10 Pro Build 1803
* Edge 42.17134.1.0 on Windows 10 Pro Build 1803

* Firefox 61.0.2 on Windows 7 Build 7600
* Internet Explorer 8.0.7600.16385 on Windows 7 Build 7600

* Firefox 61.0.1 on Linux Mint 19 Tara

Resolves #1781
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants