Skip to content

Commit

Permalink
fix iframe.js breaking links with hash URLs (#1115)
Browse files Browse the repository at this point in the history
Previously, iframe.js would not work with links contains hash URLs, i.e. links like

```html
<a id="skip-to-content-link" href="#main-content">Skip to content</a>
```

notice the URL starts with a hashtag. These links are different than normal HTML links,
in that instead of redirecting to a new page, they will scroll the browser down to the element
with the id matching the link. For the example above, it would scroll down to an element like

```html
<div id="main-content">main content!</div>
```

However, iframe.js has an issue where, when one of these links are clicked, the iframe.js will reload
and the parent frame's URL will be changed.
This issue was caused by the following process:
1. iframe.js's has a window.popstate listener, which reloads the iframe
whenever a popstate event was triggered (for instance by clicking on one of these links),
2. clicking a hash link will trigger this listener, reloading the iframe
3. reloading the iframe causes the inner iframe to send messages to the outer frame
4. these messages are received by iframe.js's onMessage listener, which then performs a history.replaceState (for certain types of messages)

To avoid this situation, this PR updates the popstate listener to be a no-op if the new URL of the page is the same as the old one, ignoring any hash URLs

J=TECHOPS-7401
TEST=manual

ran the theme's test site, created a basic iframe implementation
before this change, hash urls would have the broken behavior
after the change, they would scroll down the page without reloading the iframe or changing the parent's URL
  • Loading branch information
oshi97 committed Dec 12, 2022
1 parent 223c682 commit 68dc465
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 5 deletions.
26 changes: 21 additions & 5 deletions static/js/iframe-common.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,7 @@ export function generateIFrame(domain, answersExperienceFrame) {
// For Safari
document.body.scrollTop = 0;
});

window.onpopstate = function() {
iframe.contentWindow.location.replace(calcFrameSrc());
};

registerPopStateListener();
containerEl.appendChild(iframe);

// Notify iframe of a click event on parent window
Expand Down Expand Up @@ -135,6 +131,26 @@ export function generateIFrame(domain, answersExperienceFrame) {
}, '#answers-frame');
}

function registerPopStateListener() {
let previousLocationHref = window.location.href;

/**
* Reloads the iframe with a recalculated src URL.
* Does not reload the iframe if the URL has only changed its hash param.
*/
function popStateListener() {
/** @param {string} url */
function getURLWithoutHash(url) {
return url.split('#')[0];
}
if (getURLWithoutHash(previousLocationHref) !== getURLWithoutHash(window.location.href)) {
iframe.contentWindow.location.replace(calcFrameSrc());
}
previousLocationHref = window.location.href;
}
window.addEventListener('popstate', popStateListener);
}

/**
* Sends data to the answers iframe if possible. Otherwise the message is queued
* so that it can be sent when the iframe initializes.
Expand Down
2 changes: 2 additions & 0 deletions test-site/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ webpack-config.js
!config/global_config.json
!config/locale_config.json
!config/index.json
!config/index.ar.json
!config/index.es.json
!pages/index.html.hbs
!public/iframe_test.html
!public/overlay.html

0 comments on commit 68dc465

Please sign in to comment.