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

Adopt ClickHandler for FIE #8864

Merged
merged 4 commits into from Apr 21, 2017
Merged

Adopt ClickHandler for FIE #8864

merged 4 commits into from Apr 21, 2017

Conversation

dvoytenko
Copy link
Contributor

This is mostly straightforward refactoring, but for some reason GitHub doesn't catch the file move and shows 100% diff on document-click.js :(

Closes #6578.

src/url.js Outdated
* @param {string} url
* @return {!Location}
*/
export function parseUrlWithA(a, url) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we restrict access to this function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's nothing really dangerous about it, is there?

Copy link
Contributor

Choose a reason for hiding this comment

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

Not dangerous, but we have a cached anchor in this module to avoid every ole caller making there own. And I'd probably want to review anyone using this to make sure they intend to use a cross-origin anchor element.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense. I'll make it restricted then.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@dvoytenko dvoytenko merged commit 289f82c into ampproject:master Apr 21, 2017
@dvoytenko dvoytenko deleted the fie21 branch April 21, 2017 22:10
DarXector pushed a commit to DarXector/amphtml that referenced this pull request Apr 25, 2017
* Adopt ClickHandler for FIE

* restrict parseUrlWithA

* fixed types

* whitelist presubmits
mrjoro pushed a commit to mrjoro/amphtml that referenced this pull request Apr 28, 2017
* Adopt ClickHandler for FIE

* restrict parseUrlWithA

* fixed types

* whitelist presubmits
KenneyE pushed a commit to spotxchange/amphtml that referenced this pull request May 3, 2017
* Adopt ClickHandler for FIE

* restrict parseUrlWithA

* fixed types

* whitelist presubmits
This was referenced Dec 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants