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

Scrolling iframed specs #241

Closed
yoavweiss opened this issue Mar 15, 2019 · 6 comments
Closed

Scrolling iframed specs #241

yoavweiss opened this issue Mar 15, 2019 · 6 comments

Comments

@yoavweiss
Copy link

I'm trying to find a way to better present long-standing PRs for features which don't yet have an multi-implementation interest, so that casual reviewers can quickly evaluate the state of the the feature.

Specifically, I'm trying to do that with Client Hints, which specification spans whatwg/html#3774 and whatwg/fetch#773.

As part of that, I was thinking I could create a document which explains the different changes of the PR, but that would also present the relevant bits of the PR preview diff inline. I'm trying to use iframes to do that, but would highly prefer to load the relevant document once and be able to scroll it to the right section as the reviewer clicks on different changes.

When trying to do that from the parent frame, it is reloading the iframe every time, which is wasteful, and eventually gives me a 403 response. It would be useful for me to have a way to postMessage the PR preview, so that it will scroll to the right anchor internally, without triggering a new navigation.

Thoughts?

@yoavweiss yoavweiss changed the title Scrolling iframes specs Scrolling iframed specs Mar 15, 2019
@yoavweiss
Copy link
Author

Seems like one easy way to resolve this would be to add a message handler to https://github.com/w3c/htmldiff-nav which takes in an ID of an element and scrolls to it.

Happy to contribute such changes if y'all find it acceptable.

@annevk
Copy link
Member

annevk commented Mar 18, 2019

@domenic do you have thoughts on this? I'm on the fence.

@domenic
Copy link
Member

domenic commented Mar 18, 2019

This seems to be working around a browser bug where the browser will reload instead of scroll to the fragment, I guess? Have you looked into fixing the relevant browser bug instead?

You should not be getting 403s; do you have a repro case of that you could share? The specs are supposed to be accessible to all.

Separately I don't know what w3c/htmldiff-nav is so I'm unsure why changing it would be relevant for WHATWG specifications.

@yoavweiss
Copy link
Author

This seems to be working around a browser bug where the browser will reload instead of scroll to the fragment, I guess? Have you looked into fixing the relevant browser bug instead?

Maybe. I'm now unable to reproduce the same issue, so maybe this is not necessary.

You should not be getting 403s; do you have a repro case of that you could share? The specs are supposed to be accessible to all.

I was getting them after a while when the browser refreshed. Maybe I triggered some DoS protection along the way?

Separately I don't know what w3c/htmldiff-nav is so I'm unsure why changing it would be relevant for WHATWG specifications.

That's the diff viewer script that's integrated as part of both Fetch and HTML PR previews.

@domenic
Copy link
Member

domenic commented Mar 19, 2019

I'm not aware of any DoS protection, and would love to see an example of the 403s if you can put something online. Let us know!

@domenic
Copy link
Member

domenic commented May 12, 2019

Let's close this as it seems there's not much actionable at this time preventing the OP's use case. But, happy to reopen or create more targeted issues if we can reproduce the 403s or similar.

@domenic domenic closed this as completed May 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants