Skip to content

If page is an iframed test page, use parent's location to determine script's location#6710

Merged
aghassemi merged 4 commits intoampproject:masterfrom
aghassemi:testIframeloc
Dec 16, 2016
Merged

If page is an iframed test page, use parent's location to determine script's location#6710
aghassemi merged 4 commits intoampproject:masterfrom
aghassemi:testIframeloc

Conversation

@aghassemi
Copy link
Copy Markdown
Contributor

#6686 is blocked by this PR.

@chenshay After this PR, you should no longer need to add amp-video script to the video-players.html and we can leave the integration test to be in the backward compatibility mode.

@aghassemi
Copy link
Copy Markdown
Contributor Author

@dvoytenko As discussed, switched to using testLocation

// the test iframe is about:srcdoc.
// Unfortunately location object is not configurable, so we have to define
// a new property.
win.testLocation = new FakeLocation(window.location.href, window);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Second argument is intended to be win.

// Flag as being a test window.
win.AMP_TEST_IFRAME = true;
win.AMP_TEST = true;
// Set the testLocation on iframe to parent's location since location of
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do the same in the createIframePromise right after iframe.contentWindow.AMP_TEST_IFRAME = true;

And if possible, please also do this in RealWinFixture, right after win.AMP_TEST_IFRAME = true;.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@aghassemi aghassemi merged commit 98758ec into ampproject:master Dec 16, 2016
Lith pushed a commit to Lith/amphtml that referenced this pull request Dec 22, 2016
Lith pushed a commit to Lith/amphtml that referenced this pull request Dec 22, 2016
torch2424 pushed a commit to torch2424/amphtml that referenced this pull request Jan 3, 2017
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 this pull request may close these issues.

2 participants